Access protocol for all lists of tuples

[devinus]

This is incredibly useful for dealing with things like JSON.

For example, in my app I assumed this would work:

rpc = JsonRpc[jsonrpc: json["jsonrpc"],
              id: json["id"],
              method: json["method"],
              params: json["params"]]

Other possibilities:

star_ratings = [{1.0, "★"}, {1.5, "★☆"}, {2.0, "★★"}]
star_ratings[1.5] #=> "★☆"

[yrashk]

Wouldn't it make sense to keep a special atom clause for Keyword here?

[devinus]

Why? It's just an extra function call.

Let's try to keep access as fast as possible.

[josevalim]

I have many mixed feelings right now. :) Historically, the access protocol worked just like you showed but we have changed it with other things which makes it worthy to re-evaluate the whole situation. Here are some points of concerns:

1. The access protocol is meant for dictionaries (but we already have an exception when it comes to records)
2. Even regarding the point above, it is still not clear if keywords are considered a dictionary (you can't pass them to the Dict module) but they also work with the access protocol
3. We don't have any module that treats a general list as a dictionary. We have Keyword, but the keys needs to be atom and we are not going to lift this restriction for now (because if frames make to OTP and only atoms are allowed as keys, we could make keywords work transparently with today's keywords and frames)

In other words, everything you pass to the access protocol you should be able to pass to Dict (ideally) but this isn't true today.

TL;DR I like the idea but there are many loose ends we need to discuss before merging it. Feedback is very welcome.

[devinus]

@josevalim I'm glad you like the idea because it really cleans up a lot of my code and it seemed intuitive to me. I disagree with point 1, agree tentatively with point 3. I think the access protocol should be allowed on any structure where it makes sense. To me that includes tuple lists, dicts, keywords, records -- anything that has clear keys.

[devinus]

@josevalim What do we need to discuss?

[josevalim]

I think the access protocol should be allowed on any structure where it makes sense. To me that includes tuple lists, dicts, keywords, records -- anything that has clear keys.

This is indeed a possible interpretation of the access protocol.

In general, a protocol is a way to implement polymorphism, so ultimately you don't care to watch you are dispatching to. In this sense, it doesn't make much sense to group a record and a dict. You rarely want to mix them. Most of all, as soon you need something else (let's say, update a field), you need to rely on specific dict and records semantics, so the access protocol breaks down.

On the other hand, since the access protocol is also a syntax wise feature, we feel compelled to change its main goal to simply allowing a key based access on different structures and that is it (which was its original definition). If we go into this direction, it means that usually when you see a data[key], we can't make no further conclusion about data. It could be a dict, a list of tuples, anything.

Not supporting this approach means the code becomes (a bit) verbose, like your example above, that you need to at least convert your stars list to an OrdDict before or the json to a Binary.Dict.

In the second approach, it would also mean that it would be very hard to extend the access protocol, because the structures supported in it are too different from each other. Let's suppose we want to have an equivalent to putting a key in the access protocol, so we don't need to go through Keyword.put or Dict.put. This becomes much harder, because what updating means to a list with three element tuples?

So that's what the point 1) above boils down to. Honestly, I didn't like at all (in previous versions) the fact I couldn't reason about what data[key] means and making the access protocol broader may lead us back to this direction.

[devinus]

@josevalim I think my implementation of it for lists is just fine. I don't think it should be converted to a Dict. That loses the whole benefit of using it by converting it to an OrdDict for each access. For lists, expect a keyfind, for Dict expect a find. In my mind, the access protocol is shorthand for these kind of operations. I'm willing to bet most people are just fine with that.

[josevalim]

@devinus just for curiosity, assuming that you convert those examples to a Dict, what would be missing? You wouldn't convert it to a dict on each access, but when you create it. It makes sense, because you are using them as a dict anyway. No?

[devinus]

Also, you can't just turn it into a Dict without doing it recursively.

Example:

foo = HashDict.new [{"a", [{"b", 1}]}]
foo["a"]["b"] #=> FunctionClauseError

[devinus]

@josevalim Can we get movement on this? For the record I'm still of the opinion that this is a good change. 😸

[yrashk]

How about we write down all the pros and cons of this change and take it from there?

[devinus]

I'll keep an updated list of my thoughts here:

Pros

• Intuitive
  • It's how most users would expect it to work. Hell, I only implemented this after getting an error when it didn't work this way
• Backwards compatible
  • All code already written still works. In fact, it may even be slightly faster, which is always a plus.

[yrashk]

@devinus can you also think of cons as well?

[devinus]

@yrashk I don't see cons the same way @josevalim does. I think this makes the whole protocol simpler.

[josevalim]

I am agreeing with merging this feature. However, there is one requirement left before we merge this feature: we need to have a set of Dict functions that works on lists. The access protocol is meant to work on Dicts and by merging this feature and extending the Dict module to work with lists, we will:

1. Fix the inconsistency we have today where the access protocol applies to Keywords which are not Dicts (in the sense they don't work with the Dict module);
2. Allows us to merge this so-desired pull request;

The Dict functions that works on a list could be implemented inside the Dict module:

def get(dict, key, default // nil)
def get(dict, key, default) when is_tuple(dict), do: ...
def get(dict, key, default) when is_list(dict), do: ...

The implementation should be easy because everything is already implemented in the Keywords module. We just need to copy them over and remove the is_atom() constraints.

This brings us to the second point: what about Keyword? I am aware of the duplication of code we will have but that is meant to be temporary until we have some decision regarding frames. Keyword will be kept as atom only keys (for now) because it gives us enough space for maneuver in the all situations:

1. If frames are added - we have the choice of handling frames in the Keyword module (or not);
2. In case frames are not added - we are free to change/improve the Keyword module as we wish (as long as we are backwards compatible). For example, the Keyword module behaves as a dict because of a possible frames future. If this future does not happen, we can remove the dict behaviour from functions for faster operations. For example, Keyword.put() could simply become adding a head to the current keywords list (instead of a search and replace). After all, the Dict module would be available for strictly Dict-like operations;

With this in mind, I believe we can proceed with this. We just need the implementation for lists handling in the Dict module. So who is going to contribute it? :)