IndiffererentAccess - Adaptation of HashWithIndifferentAccess to Elixir Maps/Plug

I’ve only skimmed the implementation, but it seems as if it builds on a one-time creation of stringified/atomized counterkey with same value.

This seems to make things even worse…

One part of code updates atom key, another reads string key.

And even if you remember to re-key… Which should win if both exist?

The only solution are proper conversion to structs. No more arguing about string vs. atoms, just structs. Part of the contract, checked by dialyzer.

Yeah, to be fair this is an attempt to make the behavior backwards compatible to allow to gradually ensure the logic works, but if it were to go forward I imagine it would be a good/simple option to have to allow for replacing string keys that exist as atoms with atom keys pointing at same/indifferentized value… but I didn’t want to go too far down any path without talking and thinking about implications and tradeoffs.

As for re-keying, not sure if I understand your question/point, but it used Map.put_new so if there’s an existing atom key it never overwrites that, it only adds an atom key in addition. But maybe you meant something else?

I also had a version that left string keys with their unchanged values and only updated the value recursively on the atom key, but then thought if supporting both maybe best to do so consistently in both “paths” but, that would also make for a reasonable config/option if extending.

I still haven’t gotten a chance to really use Dialyzer since the Dialyxir updates I’ve heard have made it a lot more usable… maybe I’ll try it out on this as a simple little toy project without too much surface area, dunno… in any case, appreciate your feedback.

You will only introduce confusion by using this IMO.

What kind of a legacy problem are you trying to solve? Indeed, as @NobbZ suggested, convert the incoming controller parameters to a struct as quickly as you can so the compiler can cover part of the possible set of problems, and to be able to use Ecto as a validator as an added bonus.

What you suggest doing with this library is not just non-idiomatic; it’s terrible no matter what language you are doing it in. It increases the possible bug surface.

Let’s take a simple map, %{a: 1}, which you indifferentize. %{"a" => 1, a: 1} and then update "a", %{"a" => 2, a: 1}, now you have 2 conflicting values for the “same” key.

Your solution is not a solution, it’s changing pest for Cholera.

Always normalize the input at the edges of your system. In the system there is rarely a use case to assume non normalized data, and normalized data is nearly always a struct.

I agree I want to normalize at the edge of the system, but that’s not what I have in an existing substantial code base. I have a number of roadmap items to change some key params into an embedded_schema, which I see as helping to benefit from Ecto changeset and cast/dump behavior centralized well, but I’m not totally sold and/or educated/adapted to the idea that all params should immediately be changed into a Struct, and if I were, I don’t know what Structs those would be or how they’d change over time/which controllers would share them vs every controller having a slightly different one… (though to be fair, the bug that inspired this was a map pulled out of a param and passed on, and I suppose the same concern applies there even if the entire map isn’t treated this way, any value in it might be a map and get the same issues)… also though the point of using this would basically be that you’d only intend to work with the atom keys going forward, the string keys are there (for now) as a legacy/gradual adoption feature so its possible to drop this into existing code and not break things that rely on string keys until you’ve had a chance to move them over… but perhaps it’s better that they break loudly right away to make fixing them to use atoms a straightforward path of fixing failing tests and errors… again, I’m not at all sold on this general idea or this specific implementation, but these are good concerns to discuss for sure for anyone who does consider using it.

I also don’t think though that controller params, which is all this is intended to be used with, should really ever be updated in that sense… i think things are pulled out of them, perhaps with default values, but I don’t beleive the params map should be used wholesale as a map for any other purpose, and if only used there and never updated directly as you suggest, the “rekeying” problem is not much of a problem I think… not to say the other points aren’t valid, but, since I put this up for discussion, I’d like to make sure we’re actually discussing the same use case and reasonable concerns, not purely theoretical ones…

I was sort of surprised at the API here.

I expected to see a IndifferentMap.get function that checks for atom and string keys, while using the normal map functions for putting and updating.

Yeah the other libraries I linked to may be more like that IIRC but I wanted to try something that “just worked” transparently via a plug for some or all pipelines in an app and was easy to try out and see without littering code with references to or dependencies on it… That said, obviously people (myself included) have some legitimate qualms with this approach but I threw together quickly and may never touch again…

Oh! But maybe there’s a fun idea there for implementing instead a struct named like you suggest that implements access behavior and does that… Then this plug could initialize that struct with params map… Maybe I’ll try that as an alternate iteration! Thanks!

Have you tried to pattern match?

def has_my_atom_key(%{mykey: _} = params) do
  params
end
def has_my_atom_key(params) do
  raise Brah, "We got a problem"
end

I find thats a trivial way to catch bugs, it’s not as good as a strong type system, but it’s the best we got.

I do pattern match quite a lot! Not entirely sure how/if something like that would scale to every usage and access to a different key in a different params map across dozens of controllers, and the purpose/scope of this library/discussion is sort of looking for patterns that are a tiny bit more “automatic” and or hard to mess up/forget than something like that, but… No doubt if we’re in Elixir pattern matching probably is/could be part of the answer!