go-restli icon indicating copy to clipboard operation
go-restli copied to clipboard

Upper case known acronyms in field/variable names

Open bored-engineer opened this issue 5 years ago • 6 comments

Currently go-restli converts names to variables/exported struct fields by simply uppercasing the first letter. As some examples:

executionUuid -> ExecutionUuid
reviewBoardId -> ReviewBoardId

It would be nice if go-restli converted these names/fields to something a little more readable like ExecutionUUID or ReviewBoardID. This would require maintaining a list of known acronyms, for similar projects I've just maintained a list of known acronyms, ex:

  • ID
  • UUID
  • URL

While doing this would be nice for readability, I admit it does make any change to this list a major version bump of the library as existing clients would potentially need to migrate their struct fields. Thoughts on if this should be done?

At a high level the logic would be to split the string by each uppercase letter into a slice (ex: "reviewBoardId" becomes []string{"review", "Board", "Id"} then compare each slice member with the replacement list swapping as needed.

bored-engineer avatar Apr 23 '20 04:04 bored-engineer

Well, here's the thing, those fields were named Uuid and not UUID, so wouldn't it be kind of a departure from the schema to do that? Incidentally, our LinkedIn java code guides recommend the following:

A useful rule of thumb for acronym case is to use all uppercase for two letter acronyms and mxed case for longer acronyms (e.g. DB, Html, Url)

Though it does seem that the Go stdlib has a different opinion on that (e.g. url.URL instead of url.Url)

On the off chance that someone did something stupid and declared two fields on the same struct with the same name except that are case differences, I don't know how comfortable I feel about making that change.

PapaCharlie avatar Apr 23 '20 07:04 PapaCharlie

Haha! https://github.com/golang/go/wiki/CodeReviewComments#initialisms

While this does agree with you for human code, it does exempt machine generated code:

Code generated by the protocol buffer compiler is exempt from this rule. Human-written code is held to a higher standard than machine-written code.

PapaCharlie avatar Apr 23 '20 07:04 PapaCharlie

I guess my argument boils down to we’ve already departed from the “true” field name and introduced a (small) collision risk to export the field in the first place (making the first letter uppercase/replacing invalid characters). So if we’re comfortable with that risk, why not make the name cleaner/more Go-like too (assuming we can do it with a extremely high degree of confidence/correctness).

bored-engineer avatar Apr 23 '20 07:04 bored-engineer

@PapaCharlie What about allowing a list of acronyms to be specified as an optional in the arguments/gradle file similar to goRestli.resources? That way the default is to respect the original schema as closely as possible but if a individual client wants to make the structs more friendly they can chose to do so

bored-engineer avatar Apr 26 '20 17:04 bored-engineer

That makes me feel even less comfortable! Now if I have a shared type that is used by two different resources that were generated with different settings, the shared type won't be compatible. IMO using the same code version should always yield the same code.

But, I am slowly warming up to standardizing these things. Maybe any fields that aren't case-insensitive-unique we just slap on a CaseConflict_ prefix to the original name and move on with our lives. Would allow us to make these changes to any remaining fields

PapaCharlie avatar Apr 26 '20 17:04 PapaCharlie

I've been thinking about this some more though. What if I have a struct which only has one field: repoUrl, and we standardize it to repoURL. And then someone does something dumb, like introduce repoURL as an actual field alongside repoUrl. Now the generated code will have to generate both fields, and the identifier for the old field will actually be pointing to the new field.

PapaCharlie avatar Jul 17 '20 23:07 PapaCharlie