Skip to content
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

Usability problem with open types, optional/default fields and mapping constructors #268

Closed
jclark opened this issue Jul 25, 2019 · 5 comments
Assignees
Labels
Area/Lang Relates to the Ballerina language specification design/usability Design does not work well for some tasks incompatible Resolving issue may not be backwards compatible
Milestone

Comments

@jclark
Copy link
Collaborator

jclark commented Jul 25, 2019

Consider:

type Person record {
  string name;
  string country?;
}

Person p = { name: "James Clark", county: "Thailand" };

This will compile despite country being misspelled, because the record is open and the country field is not required. There would be the same problem if country was required, but had a default value. This seems very much not OK to me.

@jclark jclark added Area/Lang Relates to the Ballerina language specification design/usability Design does not work well for some tasks labels Jul 25, 2019
@jclark jclark added this to the 2019R3 milestone Jul 25, 2019
@jclark
Copy link
Collaborator Author

jclark commented Jul 25, 2019

I think there is a potential solution which is to do something similar to what we do with the dot operator and assignment. We do not allow p.county = "Thailand";, and I think we can use a similar rule here. The rule would be something like this: when the contextually expected type is a record type, you can only use identifiers for field names that are mentioned in the type. For other fields, you would have to use a string, e.g. if you wanted a county field you would have to do

Person p = { name: "James Clark", "county": "Bangkok" };

just as you have to use p["county"] = "Bangkok"; to do the assignment.

@jclark jclark added the incompatible Resolving issue may not be backwards compatible label Jul 25, 2019
@sanjiva
Copy link
Contributor

sanjiva commented Jul 26, 2019

I agree this is a serious usability problem and +1 for the proposed approach for solving it.

@jclark jclark self-assigned this Jul 26, 2019
@hasithaa
Copy link
Contributor

hasithaa commented Aug 1, 2019

We have implemented this for JBallerina 1.0.0-Alpha

@jclark
Copy link
Collaborator Author

jclark commented Aug 1, 2019

Is it working OK?

@hasithaa
Copy link
Contributor

hasithaa commented Aug 1, 2019

Yes, This change is a good one. It really helps to detect typos quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/Lang Relates to the Ballerina language specification design/usability Design does not work well for some tasks incompatible Resolving issue may not be backwards compatible
Projects
None yet
Development

No branches or pull requests

3 participants