Enumerable chaining #33

Closed
opened 2016-09-06 17:04:03 +00:00 by imulab · 10 comments
imulab commented 2016-09-06 17:04:03 +00:00 (Migrated from github.com)

I noticed you have Map and Select method commented out in EnumerableWithIndex and EnumerableWithKey interfaces since returning a container will require type assertion at every step, pretty ugly for chaining.

I would suggest returning EnumerableWithIndex (or EnumerableWithKey) for those methods and add a ToContainer method at the end.

This would allow API calls like:

f1 := func(index int, value interface{}) interface{} { // some mapping }
f2 := func(index int, value interface{}) interface{} { // some other mapping }
f3 := func(index int, value interface{}) bool { // some filtering }

list.Map(f1).Map(f2).Select(f3).toContainer()

At the end, the user is free to do any type assertion as they want.

I noticed you have `Map` and `Select` method commented out in `EnumerableWithIndex` and `EnumerableWithKey` interfaces since returning a container will require type assertion at every step, pretty ugly for chaining. I would suggest returning `EnumerableWithIndex` (or `EnumerableWithKey`) for those methods and add a `ToContainer` method at the end. This would allow API calls like: ``` f1 := func(index int, value interface{}) interface{} { // some mapping } f2 := func(index int, value interface{}) interface{} { // some other mapping } f3 := func(index int, value interface{}) bool { // some filtering } list.Map(f1).Map(f2).Select(f3).toContainer() ``` At the end, the user is free to do any type assertion as they want.
imulab commented 2016-09-06 17:07:29 +00:00 (Migrated from github.com)

I did notice you already have chaining tests in files like arraylist_test, but these only allow implementation level chaining, it kind of defeats the purpose of having interfaces though...

I did notice you already have chaining tests in files like `arraylist_test`, but these only allow implementation level chaining, it kind of defeats the purpose of having interfaces though...
emirpasic commented 2016-09-06 17:30:15 +00:00 (Migrated from github.com)

I agree with everything you said. I was not sure how to solve this problem and left it for some other time. I tried a few ways, but could not reach the desired effect.

How would you implement it? I am not sure I understood the ToContainer method? What does it do exactly?

Also if one returns EnumerableWithIndex (or EnumerableWithKey) then it's impossible to mix between those two, right?

I am just not sure, if I can make this work as in Ruby where you can mix maps, arrays, etc. without any type assertions, which are really ugly to that point that the library becomes useless.

Can you please make a proof of concept, either in gods or on golang playground just so I get the idea. Thanks

I agree with everything you said. I was not sure how to solve this problem and left it for some other time. I tried a few ways, but could not reach the desired effect. How would you implement it? I am not sure I understood the `ToContainer` method? What does it do exactly? Also if one returns `EnumerableWithIndex` (or `EnumerableWithKey`) then it's impossible to mix between those two, right? I am just not sure, if I can make this work as in Ruby where you can mix maps, arrays, etc. without any type assertions, which are really ugly to that point that the library becomes useless. Can you please make a proof of concept, either in gods or on golang playground just so I get the idea. Thanks
imulab commented 2016-09-07 00:19:01 +00:00 (Migrated from github.com)

Yep, I forked your repo to add and fix a few things for use of another project. The proposed enumeration changes are in there.

https://github.com/davidiamyou/go-companion/blob/master/containers/enumerable.go

You can then take a look at various data structure packages where there is the enumerable.go. Essentially, all lists now implement EnumerableWithIndex, all sets now implement Enumerable, all maps now implement EnumerableWithKey. The reason for this is that lists are by nature ordered, sets are by nature unordered (even though underlying implementation can have its elements ordered, API should not make this visible unless user does explicit type assertion himself) and maps are by nature key-valued.

The ToContainer method is really a type assertion disguised in API form. You can see in my implementations that it just returns the receiver as is. I just thought a redirection back to the Container types after chaining will be nice. It could be argued that we can change the ToContainer method to something more specific. i.e. For EnumerableWithIndex, use ToList() to allow direct access to more rich APIs than Container.

Yep, I forked your repo to add and fix a few things for use of another project. The proposed enumeration changes are in there. https://github.com/davidiamyou/go-companion/blob/master/containers/enumerable.go You can then take a look at various data structure packages where there is the `enumerable.go`. Essentially, all lists now implement `EnumerableWithIndex`, all sets now implement `Enumerable`, all maps now implement `EnumerableWithKey`. The reason for this is that lists are by nature ordered, sets are by nature unordered (even though underlying implementation can have its elements ordered, API should not make this visible unless user does explicit type assertion himself) and maps are by nature key-valued. The `ToContainer` method is really a type assertion disguised in API form. You can see in my implementations that it just returns the receiver as is. I just thought a redirection back to the `Container` types after chaining will be nice. It could be argued that we can change the `ToContainer` method to something more specific. i.e. For `EnumerableWithIndex`, use `ToList()` to allow direct access to more rich APIs than `Container`.
imulab commented 2016-09-07 00:24:59 +00:00 (Migrated from github.com)

I also returned interfaces only for New() constructors except for the trees. IMHO, users should only work with interface APIs and be shielded away from the implementation details. This is partially why, I guess, the above proposed type assertion is way easier, as it no longer have to deal with specific concrete types.

I also returned interfaces only for `New()` constructors except for the trees. IMHO, users should only work with interface APIs and be shielded away from the implementation details. This is partially why, I guess, the above proposed type assertion is way easier, as it no longer have to deal with specific concrete types.
emirpasic commented 2016-09-07 02:35:19 +00:00 (Migrated from github.com)

Thanks for finding time to explain your idea.

I found this in your commit. It looks like type assertion is still necessary or is there another actual chaining example? That's basically what I also get if I uncomment the code, but without the ToContainer() call.

I'll try to create a simpler example on golang playground and see if we can figure something out here without the ugliness of type assertions (at least not on the container types).

This is (probably unreachable) beauty of Ruby:

{a: 1, b: 2, c: 3}
  .select { |k,v| v > 2 } # {b: 2, c: 3}
  .map { |k,v| k }        # [b, c]
  .any? { |v| v == :a }   # false

I was hoping gods would be able to do something like this without a ton of type assertions.

Thanks for finding time to explain your idea. I found [this ](https://github.com/davidiamyou/go-companion/commit/6018010febf188c4d1becfdbc86be2d1e3c62103#diff-8bf445644ab2c0dff6072e811507d160R215) in your commit. It looks like type assertion is still necessary or is there another actual chaining example? That's basically what I also get if I uncomment the code, but without the ToContainer() call. I'll try to create a simpler example on golang playground and see if we can figure something out here without the ugliness of type assertions (at least not on the container types). This is (probably unreachable) beauty of Ruby: ``` Ruby {a: 1, b: 2, c: 3} .select { |k,v| v > 2 } # {b: 2, c: 3} .map { |k,v| k } # [b, c] .any? { |v| v == :a } # false ``` I was hoping gods would be able to do something like this without a ton of type assertions.
imulab commented 2016-09-07 04:38:01 +00:00 (Migrated from github.com)

Right, this is what I am talking about removing the ToContainer() method from the three Enumerable interfaces and replace with more specific methods like ToList(), ToSet() and ToMap(). If I do that, type assertion in most cases would be unnecessary.

Actually, thinking about it, I think I am gonna change my interfaces to that tomorrow. The only downside is that by doing this, I am effectively binding each of the three Enumerable interfaces to a specific kind of data structure. It would be hard later on for me to try to enumerate a map (with map entry instead of key value tuple, effectively treating it as a set), but I think it is a something I am okay with without massively changing your interface hierarchy.

I totally agree with you on the beauty of the ruby syntax. Unfortunately, I suppose the core idea of Golang is to produce maintainable and simple code by preventing people to do too much smart things.

I will post my commit here tomorrow after I have changed my interfaces and fix the test cases.

Right, this is what I am talking about removing the `ToContainer()` method from the three `Enumerable` interfaces and replace with more specific methods like `ToList()`, `ToSet()` and `ToMap()`. If I do that, type assertion in most cases would be unnecessary. Actually, thinking about it, I think I am gonna change my interfaces to that tomorrow. The only downside is that by doing this, I am effectively binding each of the three `Enumerable` interfaces to a specific kind of data structure. It would be hard later on for me to try to enumerate a map (with map entry instead of key value tuple, effectively treating it as a set), but I think it is a something I am okay with without massively changing your interface hierarchy. I totally agree with you on the beauty of the ruby syntax. Unfortunately, I suppose the core idea of Golang is to produce maintainable and simple code by preventing people to do too much smart things. I will post my commit here tomorrow after I have changed my interfaces and fix the test cases.
imulab commented 2016-09-07 05:10:22 +00:00 (Migrated from github.com)

You can find the changes to the interface in this commit

If we want things to be more flexible (for instance, capability to enumerate map as a set of entries and in the end convert that into actually a set), we would probably need to haul out something like Java's Stream API.

But for most day-to-day use cases, I would say this is sufficient.

You can find the changes to the interface in [this commit](https://github.com/davidiamyou/go-companion/commit/426c699e3ae6d4cee92611e3e44a086b8476e511) If we want things to be more flexible (for instance, capability to enumerate map as a set of entries and in the end convert that into actually a set), we would probably need to haul out something like Java's Stream API. But for most day-to-day use cases, I would say this is sufficient.
emirpasic commented 2016-09-07 17:23:55 +00:00 (Migrated from github.com)

but I think it is a something I am okay with without massively changing your interface hierarchy

I don't have too much of a problem with that, even though it's a huge task. If it would make the library more useful, I am up for it. However, I tried a few ways and could not make it work any close to Ruby.

You can find the changes to the interface in this commit

IMHO somehow these To...() calls still resemble type assertions (in essence they are exactly that). Saying ToSet() or type asserting .(*HashSet) is not that different and chaining would again be on the container super-type (lists or sets) level.

I'll have to rethink this whole thing and perhaps try uncommenting the mentioned lines to return perhaps and Iterables? Perhaps that's the part that I missed trying to return the Container. Let me play with this for a while and see if there is any way to accomplish this, I probably need to look at it from another angle. I am even at the point to be forgiving in terms of having to type assert containers during chaining, if there is no other way.

> but I think it is a something I am okay with without massively changing your interface hierarchy I don't have too much of a problem with that, even though it's a huge task. If it would make the library more useful, I am up for it. However, I tried a few ways and could not make it work any close to Ruby. > You can find the changes to the interface in this commit IMHO somehow these To...() calls still resemble type assertions (in essence they are exactly that). Saying `ToSet()` or type asserting `.(*HashSet)` is not that different and chaining would again be on the container super-type (lists or sets) level. I'll have to rethink this whole thing and perhaps try uncommenting the mentioned lines to return perhaps and Iterables? Perhaps that's the part that I missed trying to return the Container. Let me play with this for a while and see if there is any way to accomplish this, I probably need to look at it from another angle. I am even at the point to be forgiving in terms of having to type assert containers during chaining, if there is no other way.
imulab commented 2016-09-07 19:25:42 +00:00 (Migrated from github.com)

You are right, it is still a type assertion in the end. Because enumerables have their own interfaces, I just cannot get away with type assertion without having to merge enumerable interfaces with data structure interfaces.

Please let me know if you have come up with anything, I would love to incorporate it into my project later.

At the meantime, I will take a look at the possibility of mimicing Java's Stream API.

You are right, it is still a type assertion in the end. Because enumerables have their own interfaces, I just cannot get away with type assertion without having to merge enumerable interfaces with data structure interfaces. Please let me know if you have come up with anything, I would love to incorporate it into my project later. At the meantime, I will take a look at the possibility of mimicing Java's Stream API.
emirpasic commented 2018-09-19 01:52:31 +00:00 (Migrated from github.com)

i still don't have an idea how to solve this, closing after two years...

i still don't have an idea how to solve this, closing after two years...
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
golibs/gms-gods#33
No description provided.