freckle / guides

Freckle guides and best practices
MIT License
53 stars 7 forks source link

Do not prefix record field names #89

Closed chris-martin closed 9 months ago

chris-martin commented 9 months ago

~I'd like to propose we enable OverloadedRecordDot by default everywhere.~

I'm proposing the following changes to the style guide:

I've been using this extension set in various isolated places. I wouldn't say for sure there is consensus on this, but it seems like we're headed that way...

If you need to see what this looks like in action, browse the content-client repo, which uses this approach extensively.

Advantages

The main advantage is decluttering code that can get very noisy when names are too long. Personally I find the long names a real impediment to understanding, especially when both the prefix and the name are multi-word phrases.

Not using prefixes often makes it easier to map Haskell types to JSON schemas. When defining an API type, often the record field names can exactly match the JSON field names.

Drawbacks

If you destructure a record with NamedFieldPuns or RecordWildCards, it still does bring the name into scope where it can conflict with other identifiers. This comes to light most readily with fields named id, since many of the records we deal with have an ID field, and id is a function defined in every prelude. Usually this means that I do not want to bring fields into scope in this way, and instead rely quite heavily on OverloadedRecordDot. You can also get around the issue by binding an unambiguous local name using ordinary record syntax (let Student{ id } = student vs let Student{ id = studentId } = student).

OverloadedRecordDot does not work with fields that have higher-rank types. This means that if you have a type like newtype Storage = Storage{ run :: forall a. Operation a -> IO a }, you cannot access that field by writing storage.run. This is not a huge deal for us since we do not often have records like this.

pbrisbin commented 9 months ago

I agree we are heading in this direction, but is there any reason not to just open up the discussion on the whole shebang?

Is there any downside to only enabling this, vs doing NoFieldSelectors, removing RecordWildcards, etc, in one go?

chris-martin commented 9 months ago

I think NoFieldSelectors is a lot of work, whereas enabling OverloadedRecordDot everywhere is just an immediate ergonomic improvement.

But yes, we should have that discussion. I'm :+1: on it.

pbrisbin commented 9 months ago

Yeah, I don't think we have to consider how much work it is when talking about Guides. Here we're just documenting our decision / intent. So if we're all in, might as well document it that way?

chris-martin commented 9 months ago

Okay, I'll turn this into a bigger proposal...