New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Atom key'd maps #11

Closed
meyercm opened this Issue Jun 5, 2016 · 13 comments

Comments

Projects
None yet
4 participants
@meyercm

meyercm commented Jun 5, 2016

I've seen the discussion on why ShortMaps defaults to string keys, and while I personally am not swayed, I can respect that it isn't my project. Frankly though, I use atom key'd maps and structs all the time -- and because this macro is brilliant, I've just copy-pasted the code and changed @default_modifier in basically all of my apps, rather than using the Hex package and adding the trailing a.

Referencing discussion in the mailing list, irc, and issue #10 as evidence for how badly some folks (including me) despise the a, how do you feel about using ~m for string keys and ~M for atom keys? While the consistency with ~w/~W is slightly diminished, I think it's a decent compromise position. Additionally, this macro doesn't support interpolation, so the consistency with ~w is already sort of broken.

e.g.:

foo = 1
bar = 2
~m{foo bar} == %{"foo" => 1, "bar" => 2}
~M{foo bar} == %{foo: 1, bar: 2}
@whatyouhide

This comment has been minimized.

Owner

whatyouhide commented Jun 5, 2016

I am not a fan of the ~m/~M solution because I think modifiers are provided in Elixir exactly for this use cases. ~m() and ~m()a do the same thing. I think probably Elixir could have gone with ~w() and ~w()R (for "R"aw) as well tbh, but that's another discussion :). That said, I am strongly convinced that a one character difference will not (and should not) matter when using this sigil, especially since I strongly suspect that atom keys in maps is not a very high percentage of the usage, maybe 50% or around that; Phoenix params, for example, use string keys, and that's a lot of use cases :)

I'd like to ping @josevalim as well on this just to hear his opinion, but I lean on 👎 for this change :)

@meyercm

This comment has been minimized.

meyercm commented Jun 6, 2016

I'm aware of the phoenix use case, but outside of web interactions, I can't justify prioritizing string-key maps: Structs must have atom keys, and the map syntax itself is slanted toward atoms, as it provides the shorthand notation <atom>: value. Here, though, atom keys are second-class citizens, which is inconsistent with the rest of the language - an issue that I think is a bigger deal than incompatibility with the ~w sigil, tbh.

In grepping one codebase of mine (~18k loc), I found 8 of my ~1800 maps have a string literal key. Perhaps my use isn't particularly normal, so I grepped a few others:

Ecto: (v1.0.6) 8 string literal keys, 865 maps
Postgrex: (v0.9.1) 2 string literal keys, 190 maps
Poison: (1.4) 4 string literal keys, 12 maps
Decimal: (1.1.0) 1 string literal key, 146 maps
Plug: (0.14.0) 14 string literal keys, 278 maps

This selection is just what I happened to have on hand, from a seriously out-of-date deps directory, and uses a very hasty methodology; but I think illustrates well that string literal keys are perhaps not as common as you might think.

Like I said before, I can respect that it's your project, and you can do what you want; however, the a doesn't cut it for me. I really wish that @josevalim would have gone with baked-in support via %{atom, other_atom} rather than the sigil, but again, it's his project, and I'm happy to benefit from it.

Methodology for counting:

  • string literal keys: grep -r '\" =>' | grep -v test | wc -l
  • maps: grep -r '%' | grep -v test | wc -l
@meyercm

This comment has been minimized.

meyercm commented Jun 6, 2016

This issue is pretty important to me, so at this juncture, I think my only tenable choice is to publish another hex package that provides the short atom maps. I said it before, but just so I'm clear: I'm super impressed with your code, I'm just not willing to sacrifice readability for abstract semantics that I personally think are incorrect.

@meyercm meyercm closed this Jun 6, 2016

@whatyouhide

This comment has been minimized.

Owner

whatyouhide commented Jun 6, 2016

@meyercm this being open source, you're absolutely free to do whatever you want. This code's license is UNLICENSE so you can even copy this whole repo an not point to the original one/mention me as the original author, and it would be perfectly fine 😃

However, for the sake of discussion, I would argue that for the benefit of our community, one single package that does this would be likely better than more than one. For example, we could all focus our design and development efforts into making short_maps the de facto library for ~m sigils, to the point where if I see ~m in some Elixir code, I'm almost sure it comes from short_maps. I'm open to other inputs on ways to make things simpler/more concise, even if I don't believe ~m/~M or ~m() with atoms by defaults are good solutions (but my mind can be changed of course!).

Again, I find it hard to wrap my head around the fact that a single a after the sigil (backed up by ~w as well, so not completely extraneous in Elixir) can

sacrifice readability

Is it really a "sacrifice"? For example, say I hipotetically would prefer System.cmd/3 to be called System.command/3 because I prefer explicitness; I could try and convince the Elixir team that it's better until the language is changed, or I could provide a Hex package that provides System2.command/3 (bad example but bear with me), or I could just accept System.cmd/3 and try to focus on other things that could make Elixir better (say, for example, making the implementation of System.cmd/3 better in some way). Do you see my point?

The last thing I want is to sound condescending/lectury/annoying with this comment; again, all I care about is the well-being of our community. :) I will say this again, you're absolutely free to do whatever you want with the code in this package and to release anything you want on Hex (provided it follows Hex's guidelines, of course 😄).

@meyercm

This comment has been minimized.

meyercm commented Jun 6, 2016

So really, above all, thanks for being reasonable about this discussion, even when 'some guy on the internet' is being whiny about an issue that's been raised and closed over and over.

The last thing I want to do is waste time maintaining a support library that is literally a duplicate of someone else's: I have many better things to spend my time on (I don't have any Hex packages precisely because I haven't found an unfilled need), and fracturing the user-base is the opposite of what I want- as much as I hate writing %State{id: id, status: status, monitors: monitors} = state over and over in my GenServers, I hate reading it more, and I'd love to see the community embrace ShortMaps.

I think part of the disconnect between our viewpoints is that you see ~m(<string>) as a sigil, and so making it consistent with ~w(<string>), also a sigil, is of importance to you. The postfix modifier syntax for sigils is a part of sigil-dom, so it makes sense to you that the a on the end isn't a big deal.

Here's my perspective: Elixir is my language of choice, and as a professional developer, I write Elixir for 8 hours in the office, then come home and goof around for longer on my personal code. After I was exposed to ES6 map syntax, I had the same sort of revelation that came with grokking Erlang pattern matches... I really want %{mykey} to just work, but based on the previous discussions in the mailing list, that isn't going to happen.

When it comes to this subject, ~m isn't a sigil for me. ~m is a way to fix the language. One of the best parts of sigils is the ability to use a large set of delimiters: ~r/regex/ is awesome because it looks just like a regex should. Similarly, you'll note that my code examples always use ~m{}, because the curly braces look just like a map should - and that's why I want to avoid the a: not because one character is such a big deal, but being at the end, the whole construct stops looking like a map. (On the exact same grounds, I would advocate for splitting the string on ,, and trimming whitespace, but one battle at a time)

Frankly, I spend all day in Elixir, so quality of life improvements are important to me. This article from Joel on Software is a classic, but the point is right on, and the part about the bakery is where I'm coming from. When I get to replace a map pattern match with 4 or 5 variables, cutting a multi-line statement down to one: that's a small victory. Thats why my codebases all have Utils.ShortMaps, which is identical to yours except for line 2.

I don't want to belabor the point any further, but I do want a unified solution, not a fragmented one, and I want to be able to proselytize about this library. I hope something I've said changes your mind.

Again, thanks for being reasonable and open to discourse.

@whatyouhide

This comment has been minimized.

Owner

whatyouhide commented Jun 9, 2016

Discussing is always a pleasure, no worries. Yes, definitely the core idea where we don't agree is seeing ~m as a sigil vs. a way to emulate ES6 syntax. To me, there's little gain in having ~m{foo, bar} to resemble ES6 because I don't care about writing ES6; ~m{foo, bar} is even equally verbose to me than ~m(foo bar)a. It may look more familiar if one is used to ES6, but sigils look perfectly fine to me and feel less "extraneous" in the language. Anyways, if you go with ~m{foo, bar} and such syntax, then it makes much more sense to have a separate package for this as it will diverge pretty substantially from short_maps, at least on the design level :).

Thanks for the discussion, don't hesitate to open such "inflammatory" issues again as far as I'm concerned because often they lead to interesting talks :)

@meyercm

This comment has been minimized.

meyercm commented Jun 9, 2016

Commas in the sigil would be nice, just like ice cream and warm cherry pie- it's the trailing a which I actually care about. I put this commit together, which provides the ~M{} sugar for ~m()a without impacting any other functionality, and I'm completely satisfied with it, with no need to touch it until a bug comes along.

To clarify one point, my goal isn't to emulate ES6 syntax, but rather to emulate Elixir map syntax, while retaining the benefit of the ES6 syntax, namely the redundancy elimination.

Right now, I'm wrestling with the right way to move forward; I have another library meyercm/flub that depends on the ~M{} syntax, but as far as I can tell, once I publish ShorterMaps to Hex (I haven't), I can't take the package down, and I'm committed to division. If I'm just going to pull my deps from github, I think I might delete my alternately named repo, and fork this one. On the other hand, when I go to publish Flub, I'd rather that all of it's deps come from Hex, rather than pulling from github in the mix.exs. Thoughts?

@whatyouhide

This comment has been minimized.

Owner

whatyouhide commented Jun 9, 2016

@meyercm nope, I have no better idea for that. You definitely shouldn't publish a package that depends on GH repos (I suspect you actually can't do that at all), but I see your point in publishing shorter_maps and then being unable to take it down. I have no good suggestion 😕

@sjmueller

This comment has been minimized.

sjmueller commented May 31, 2017

It's really unfortunate that the split happened. Elixir has far fewer libs than the nodejs ecosystem, but the advantage is suppose to be that they are of higher quality, with significantly less fragmentation. I know this is quite retrospective, but the lack of compromise here has diminished this advantage.

Hopefully in the future, the community will strive towards unity over fragmentation.

@whatyouhide

This comment has been minimized.

Owner

whatyouhide commented Jun 1, 2017

@sjmueller I agree wholeheartedly with that. I really really want to try and have one good library for many use cases rather than many libraries for few use cases. In this case, shorter_maps has been published so the division happened in a basically irreversible way. Compromise was hard for me here because I strongly disagree with the syntax used in shorter_maps: as I expressed above, I really really feel that there's no point in making things look like ES6 in a whole different language. So sadly I don't see how else this could have ended up; I hoped that one of the two packages would become predominant but right now they have the same order of magnitude of downloads on Hex so community is not expressing a clear preference.

@meyercm

This comment has been minimized.

meyercm commented Jun 1, 2017

@sjmueller -

It's really unfortunate ...<snip>... Hopefully in the future, the community will strive towards unity over fragmentation.

might be overstating things a bit. ShorterMaps and ShortMaps aren't build toolchains. They aren't package managers, or even core libraries that everyone uses. They are niche syntax sweeteners and tbh, a little bit of exploratory divergence is a good thing. If we had gone with my simple compromise:

~M{var var2 var3} === ~m(var var2 var3)a

my freedom to improve ShorterMaps would have been lost, and the v2 features would never have emerged, syntax like:

~M{old_map|key, _other_key, third_key: third_key_actual_variable}

At the end of the day, ShorterMaps is out there to please myself, and I offered it to the community in hopes that it would please others (and so I could use it as a dep for other packages). One great benefit of free software is that we can shape it to match our own needs.

@whatyouhide -

there's no point in making things look like ES6 in a whole different language

I can think of two great reasons: a) making it easy for other developers to participate in your community, and b) reusing someone else's good ideas to make your language better.

Look at how many languages stole syntax from C: curly braces, parenthesized if conditions, and semi-colon line endings, just to name a few. I'm sure Java had better market penetration because developers thought it looked like the C/CPP they'd been writing for the last dozen years. Think about how many Ruby developers jumped onto Elixir just because we use do ... end blocks.

Novel syntax constructions are rare, but for example: Elixir took Erlang's multi-head function definition not because it had to, but because it is awesome syntax. What would Elixir be without it? How often do you wish <insert your other languages here> had multiple function heads?

I find it quite pleasant to use ES6 map syntax - it makes me smile to eliminate repetitive code, and for me, that alone is enough reason to emulate it.

I'm still super grateful for the ShortMaps package for the great start- and I can appreciate that you have strong feelings about the syntax; so do I.

@sjmueller

This comment has been minimized.

sjmueller commented Jun 1, 2017

I get that it doesn't seem like a big deal, but if everyone strongly disagreed over these types of matters, then a much needed advantage becomes lost over other larger communities who have more momentum.

For the record, I chose shorter_maps over short_maps for the better syntax and v2 features, so I am grateful that you were able to express yourself and take things forward. I just wish it was in one lib, for the sake of the community. Hopefully other lib authors with disagreements can learn from what happened here.

@michaeljbishop

This comment has been minimized.

michaeljbishop commented Oct 5, 2017

I have to say, even though it's clear there are strong disagreements in this thread. I am very impressed at the civil tone. I also need to add that I just grepped through our code and saw that we use atoms 100% of the time. We also limit our usage to passing params into our unit tests from the setup clause.

That said, I prefer atoms as the default as well. I think it's because we use it with atoms so consistently that when when ExUnit initially won't even run my test, it's almost always because I forgot to include the training a and it breaks my flow to fix it.

I'd love to see a code-breaking v2.0 of this that would do the following:

  1. Accept /s,/a, and /c as modifiers (still consistent with ~w), but make /a the default. You can use /s if you want strings.
  2. Add /t that would turn the result into a MapSet of values.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment