-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
Normalize struct names #37
Comments
Hmm... @jiegillet is it possible that we accidentally implemented this in #44 ? |
Yes, it's entirely possible :) I don't necessarily think that we should normalize all map keys indiscriminately, but we certainly can do it for the ones in defstruct [:placeholder_1, :placeholder_2] # or whatever numbers @angelikatyborska what do you think? Quick example: in |
I'm having a hard time coming up with a good rule about what to normalize when with regard to map, struct, and keyword list keys. In general, it seems to me like key names are just like variable names and should be normalized. But then I start coming up with a bunch of exceptions: keys in structs that come from the standard library are always known and shouldn't be normalized. Keys in some special usage keyword lists like I'm currently writing a bunch of more tests that just pass to highlight the current behavior: https://github.com/exercism/elixir-representer/pull/73/files For example struct names are only kept if they're not user defined, which is great. And you're right about this bug:
|
@iHiD advised on Slack:
|
I know this isn't the point of this discussion, but removing import statements entirely is potentially quite a good idea. (F# does this: https://exercism.org/docs/tracks/fsharp/representer-normalizations#h-remove-import-declarations) |
@iHiD What's the reasoning behind removing imports? I suspect they might not apply in Elixir. |
I'd sort of flip the question and ask what's the reason for keeping them? The solution being normalised passes the tests, so it's importing whatever it needs to import. So the question becomes are they things you'd ever want to give feedback on?
My instinct is that they're not things that we need to give feedback on automatically, so increasingly the likelihood of normalisation feels better. |
Most of the time, yes. I can see those scenarios for imports in Exercism exercises:
|
That's a good idea, let's order imports :) |
@jiegillet I take your comment to mean that you agree that imports should stay in the representations 😁 I created an issue about ordering them: #75 |
While those are solid examples, all three of them feel like they could be really good analyzer checks that would cover all exercises and then allow the representations to be further normalised and then reduce the amount of feedback given manually in the automation UI. But I'm entirely happy to defer to whatever you decide - you know the range of possibilities in Elixir much better than me! 🙂 |
This is mentioned in #29.
Struct names should be replaced by placeholders as well.
Example input
Desired output
I think
__MODULE__
should not be replaced, because it means different things in different modules. It's already normalized, in a way...Current output
The text was updated successfully, but these errors were encountered: