mrange / cpplinq

LINQ for C++ (cpplinq) is an extensible C++11 library of higher-order functions for range manipulation. cpplinq draws inspiration from LINQ for C#.
http://cpplinq.codeplex.com/
Other
182 stars 39 forks source link

Added an aggregate operator without a seed #3

Open zwvista opened 9 years ago

zwvista commented 9 years ago

I've implemented the following operators aggregate without a seed last element_at distinct with a key selector start_with to_lookup with a value selector

mrange commented 9 years ago

Hi. Been a bit busy. I am planning to take a closer look at this the coming week.

mrange commented 9 years ago

So a few comments. Overall I think it looks good and should be mergable. Have you done code coverage runs to see the code-coverage of the new code? I guess I really should add some proper CI...

mwpowellhtx commented 9 years ago

Sorry to inject my two cents late in the game. This is similar to LINQ Aggregate? That's with/without a seed? For instance, without, like gathering some std::string elements? Or with, like doing a summation, multiplication, or division, and injecting either 0 or 1, for example?

zwvista commented 9 years ago

@mwpowellhtx Yes, the reduce operator is just like the LINQ Aggregate without a seed. I had to choose another name because the selector version of the reduce operator has exactly two parameters. Had it been another overload of the aggregate operator, it would not be differentiated from the aggregate operator with a seed. Does it make sense?

zwvista commented 9 years ago

@mrange Thanks for your detailed review, from which I've learned a lot. I'm not sure whether I have done a complete code coverage run of the new code. Simply don't know how to do it. But I've tried my best to add tests for all the new code.

mwpowellhtx commented 9 years ago

@zwvista I understand, without seed is what you did. What is the "default seed"? I think users need/want the ability to decide for themselves what that seed should be. I know I do: i.e. in the couple of instances I explained above; contrast multiply/divide with add/subtract, etc, etc. Zero (0) would not work for multiply, where as One (1) is probably inappropriate for add. Make sense?

mwpowellhtx commented 9 years ago

@zwvista I just got through putting the C++ transform numerics range algorithm, as a matter of fact, along similar lines. Have a look at, transform as a frame of reference. The output type is the seed, and the binary operator accepts an Output and and Input, and returns an Output. Along the same lines as LINQ Aggregate.

zwvista commented 9 years ago

@mwpowellhtx You don't really need a seed when you do summation or multiplication. For example, if you want to know the sum or the product of an array of numbers such as [2, 3, 4], the only thing you need to do is compute like this: 2 + 3 + 4, or 2 * 3 * 4 and you will get the result. You simply don't need a seed like 0 or 1. In fact, the first element in the array will serve as the default seed in an aggregate without seed. In such an algorithm, what you must be sure of is that the array is not empty, or you'll get an error. That's it.

And the C++ algorithm function that does the same work as Aggregate in LINQ is not transform. It's accumulate, if I'm not mistaken. The accumulate function in C++ always comes with a seed(init) value. We don't have an accumulate without seed in the standard library. But that doesn't mean an accumulate without seed is useless.

mwpowellhtx commented 9 years ago

@zwvista That's not the point. I use addition or multiplication as illustrations only. The point is that the end-user should be telling aggregate what the seed should be, not the other way around. I'll grant you there is one version of LINQ Aggregate that does not specify seed. Fair enough. However, there is another version that does, and should, accept a seed. Hope this helps. Moving on...

zwvista commented 9 years ago

@mwpowellhtx Sorry, but I just can't get your point. Why do you think the end user should always tell aggregate what the seed is and not the other way around? The end user should have the choice not to offer a seed in case there is no need to do so. An aggregate without a seed is not an odd concept. In an aggregate algorithm like maximum or minimum, you are even not supposed to give a seed because there is no meaning in doing so. The seed simply doesn't exist because there is no answer to a question like: What is the biggest/smallest element in an empty sequence?

In an aggregate with a seed, the return type is same as that of the seed. Without a seed, there is still no problem in deciding the return type of the algorithm. Just take the type of the element of the sequence, and it's done. There is no such algorithm/overload in the standard library of C++ but as you know, there is one in LINQ. So is there in F# and Java 8.

Obviously Human Interface is the approach taken in the API design in LINQ and cpplinq. That is, to provide as many algorithms/overloads as possible to facilitate the use of the library. Surely you can do without an aggregate without a seed, but you can also do without many of the APIs here.The aggregate without a seed is shorter and will be useful in some context. That's the key point of the Human Interface. That's also my two cents.

mwpowellhtx commented 9 years ago

@zwvista If you're going to provide an aggregate extension, along the same lines as LINQ, and not dissimilar to the C++ range-based transform function, then I think you provide both: one with seed, one without. I tend to prefer with seed, because it's not up to the API to dictate what that result should look like, that's up to the end-user to decide that.

@mrange As repository moderator, will defer to you on this one. I've stated my peace and will leave it at that.

I've said my peace. What you do with it is up to you.

Thank you...

zwvista commented 9 years ago

@mwpowellhtx I'm sorry if I've misunderstood you. I'm afraid you have missed something. I don't need to provide the aggregate with a seed because it is already in cpplinq long before this thread. It's just called aggregate. Do consult the documentation.

zwvista commented 9 years ago

In order to simplify the interface, I've removed the reduce operator and replaced it with a third overload of the aggregate operator. So now the aggregate in cpplinq is just like its LINQ counterpart.

mwpowellhtx commented 9 years ago

Ah @zwvista then it is I who have misunderstood.