5

I have

typedef std::vector<int> IVec;
typedef std::vector<IVec> IMat;

and I would like to know how I can fill an IMat by using std algorithms, ie how to do the following with less code (all the IVecs have the same size) ?

void fill(IMat& mat){
    for (int i=0;i<mat.size();i++){
        for (int j=0;j<mat[i].size();j++){
            mat[i][j] = i*j;
        }
    }
}

PS: already a way to fill the matrix with a constant would help me. And preferably with pre-C++11 algorithms.

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185
  • 2
    Sice the values depends on the indices ... just stay with the loops. – deviantfan Oct 22 '15 at 11:48
  • 1
    Do you need to initialize empty vectors, or fill in the values of vectors that are already not empty? Also, do you want to fill them with the index-based values (`i*j`), or any value (e.g. `0`) will do? – SingerOfTheFall Oct 22 '15 at 11:49
  • @SingerOfTheFall Not sure if I understand your first question, my `IMat` is already initialized, i.e. not empty. Yes, the values should depend on the indices. However, it would already help me to fill it with a constant. – 463035818_is_not_an_ai Oct 22 '15 at 12:08
  • For fun, I attempted this without explicit looping or `for_each`, and managed to do it with nested `std::transform`. http://coliru.stacked-crooked.com/a/96f803182ad4abad Technically you could put it all on one line to make it "less code", but why would you? The simple loops in the OP are much cleaner. – AndyG Oct 22 '15 at 12:20
  • @AndyG I have to admit, that the only reason I asked this question was to get a better understanding of how to use algorithms. Of course, the code I posted is completely fine, but I wasnt sure if there is a more elegant way to write it. Seems like there isnt and moreover all suggested solutions are using lambdas, while I have to code pre-C++11. It was worth a try ;) – 463035818_is_not_an_ai Oct 22 '15 at 12:25
  • @tobi303 While I agree that simple loops are the best here, every lambda can be replaced by a object of a class with `operator()` implemented – deviantfan Oct 22 '15 at 12:35
  • @deviantfan yes, I know, but this usually results in more code rather than less. Afaik, this was the reason to introduce lambdas. – 463035818_is_not_an_ai Oct 22 '15 at 12:52

3 Answers3

5

The best solution is the one that you have already implemented. It takes advantage of using i/j as both offsets and as inputs to compute the algorithm.

Standard algorithms will have to use iterators for the elements and maintain counters. This data mirroring as a sure sign of a problem. But it can be done, even on one line if you wanna be fancy:

for_each(mat.begin(), mat.end(), [&](auto& i) { static auto row = 0; auto column = 0; generate(i.begin(), i.end(), [&]() { return row * column++; }); ++row; });

But as stated just cause it could be done doesn't mean that it should be done. The best way to approach this is the for-loop. Even doing it on one line is possible if that's your thing:

for(auto i = 0U;i < mat.size();i++) for(auto j = 0U;j < mat[i].size();j++) mat[i][j] = i*j;

Incidentally my standard algorithm works fine on Clang 3.7.0, gcc 5.1, and on Visual Studio 2015. However previously I used transform rather than generate. And there seem to be some implementation bugs in gcc 5.1 and Visual Studio 2015 with the captures of lambda scope static variables.

Community
  • 1
  • 1
Jonathan Mee
  • 37,899
  • 23
  • 129
  • 288
  • if it fits, putting stuff on one line is very much "my thing" but if I had to decide, omitting the `{}` around the body of a for-loop would be forbidden ;) – 463035818_is_not_an_ai Oct 22 '15 at 13:48
  • @tobi303 Since newlines are free I use them liberally in my personal code. (Along with curly braces.) I think it makes it more readable. But at the end of the day, I think that the most important decision in this case is not whether to do it on one line but to do it in a `for`-loop. – Jonathan Mee Oct 22 '15 at 13:53
  • 2
    +1 for this statement alone: "But as stated just cause it could be done doesn't mean that it should be done" – Richard Hodges Oct 22 '15 at 13:54
3

I don't know if this is better than a double for loop, but one possible way you could do it using STL in C++11 would be using two for_each as follows:

int i(0);
std::for_each(mat.begin(), mat.end(),
[&i](IVec &ivec){int j(0); std::for_each(ivec.begin(), ivec.end(), 
                           [&i,&j](auto &k){k = i*j++;}); ++i;});

LIVE DEMO

101010
  • 41,839
  • 11
  • 94
  • 168
  • 2
    I'm pretty sure the order of generate is not guaranteed => depending on the implementation, you're filling the wrong values. – deviantfan Oct 22 '15 at 12:13
  • @deviantfan didn't know that, surprise surprise then. – 101010 Oct 22 '15 at 12:19
  • 1
    @deviantfan Can you give any proof to support that? http://en.cppreference.com/w/cpp/algorithm/generate#Possible_implementation implements it as: `while (first != last) *first++ = g();`. I was of the belief that *all* iterator algorithms were sequential. There could be a lot of problems, with lambdas for example if that were not the case. – Jonathan Mee Oct 22 '15 at 12:34
  • 1
    @JonathanMee Anywhere the standard doesn't specify an ordering on an algorithm, you should assume that an implementation can exploit that say for parallelism. – 101010 Oct 22 '15 at 12:35
  • @101010 Does the standard specify anything about `for_each`? Thankfully the standard implementation of parallel algorithms is being maintained in a separate library: http://en.cppreference.com/w/cpp/experimental/parallelism/existing#generate – Jonathan Mee Oct 22 '15 at 12:37
  • @JonathanMee Can you proof the opposite? Anything not specified is not specified, and you linked a "Possible implementation". – deviantfan Oct 22 '15 at 12:38
  • 1
    for_each is ordered (begin to end, see §25.2.4.2). And why unordered stuff is a problem for lambdas? – deviantfan Oct 22 '15 at 12:38
  • @deviantfan The manipulation of an external variable makes it problematic. Any `generate` function called which reads and writes an external variable would have non-deterministic results. Thankfully [gcc](https://gcc.gnu.org/onlinedocs/gcc-4.6.2/libstdc++/api/a01045_source.html#l05012) and Visual Studio both implement this as a standard `for`-loop. And since parallelism is limited to another library I don't expect that to ever change. – Jonathan Mee Oct 22 '15 at 13:14
  • @deviantfan I've opened a question [here](http://stackoverflow.com/q/33283275/2642059) cause I'd like clarification on this. – Jonathan Mee Oct 22 '15 at 14:19
  • @101010 Ordering is specified as part of InputIterator. – curiousguy Oct 28 '15 at 12:46
  • @curiousguy order of execution is not explicitly guaranteed in the standard. This gives an implementer the liberty to define the execution order of an STL algorithm the way she thinks proper as long as the final result conforms to what the standard mandates. The concept of 'InputIterator' doesn't dictate the execution order but rather the access order of the container/range. These two are different things. – 101010 Oct 28 '15 at 16:08
  • @101010 You mean, the algorithm could dereference the iterator, make temporary copies, apply the functor on the copies...? – curiousguy Oct 28 '15 at 16:49
  • @curiousguy imagine the simplest thing, iterating through the range and dispatching each element along with the operation to a separate thread. And right after the loop joining the threads. – 101010 Oct 28 '15 at 17:06
  • @101010 So the result of `*it++` is copied before the functor is applied (in `for_each` and `transform`). – curiousguy Oct 28 '15 at 18:39
  • @101010 Thought you might be interested [deviantfan changed his opinion, and is now on board with `generate` being sequential](http://stackoverflow.com/questions/33283275/is-generate-guaranteed-to-be-executed-sequentially#comment54412577_33305627). Which I believe is the correct position. – Jonathan Mee Oct 30 '15 at 11:29
  • wouldnt it be a conformorming (though not very useful) implementation, to iterate sequentially from beginning to end, store pointers to the elements in the container and only then perform the algorithm in random order? – 463035818_is_not_an_ai Mar 22 '17 at 20:01
3

Just thought I'd comment further on Jonathan's excellent answer.

Ignore the c++11 syntax for now and imagine that we had written some supporting classes (doesn't matter how for now).

we could conceivably come up with code like this:

auto main() -> int
{
    // define a matrix (vector of vectors)
    IMat mat;

    // resize it through some previously defined function
    resize(mat, 10, 10);

    // get an object that is a pseudo-container representing its extent
    auto extent = extent_of(mat);

    // generate values in the pseudo-container which forwards to the matrix
    std::generate(extent.begin(), 
                  extent.end(), 
                  [](auto pxy) { pxy.set_value(pxy.x * pxy.y); });

    // or even

    for (auto pxy : extent_of(mat)) {
        pxy.set_value(product(pxy.coordinates()));
    }

    return 0;
}

100 lines of supporting code later (iterable containers and their proxies are not trivial) and this would compile and work.

Clever as it undoubtedly would be, there are some problems:

  • There's the small matter of the 100 extra lines of code.
  • It seems to me that this code is actually less expressive than yours. i.e. it's immediately obvious what your code is doing. With mine you have to make some assumptions or go and reason about the extra 100 lines of code.
  • my code needs a lot more maintenance (and documentation) than yours

Sometimes less is more.

Richard Hodges
  • 68,278
  • 7
  • 90
  • 142