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

Dart:html should use lowerCamelCase for constants in Dart 2.0. #31367

Closed
lrhn opened this Issue Nov 14, 2017 · 9 comments

Comments

Projects
None yet
6 participants
@lrhn
Member

lrhn commented Nov 14, 2017

The Dart style guide recommends lowerCamelCase for constants. Dart platform libraries used to use SCREAMING_CAPS.
The platform libraries are changing to the recommended style for Dart 2.0, and dart:html and other web-facing libraries should do that too.

@lrhn lrhn added this to the 2.0 milestone Nov 14, 2017

@dgrove

This comment has been minimized.

Show comment
Hide comment
@dgrove

dgrove May 10, 2018

Member

I am under the impression that this is unlikely. @terrylucas ?

Member

dgrove commented May 10, 2018

I am under the impression that this is unlikely. @terrylucas ?

@leafpetersen

This comment has been minimized.

Show comment
Hide comment
@leafpetersen

leafpetersen May 10, 2018

Member

Terry and I discussed it a while back and I think the conclusion was that this one is staying (and probably should in the name of consistency with the JS names). @terrylucas if I'm remembering wrong feel free to re-open.

Member

leafpetersen commented May 10, 2018

Terry and I discussed it a while back and I think the conclusion was that this one is staying (and probably should in the name of consistency with the JS names). @terrylucas if I'm remembering wrong feel free to re-open.

@nilsdoehring

This comment has been minimized.

Show comment
Hide comment
@nilsdoehring

nilsdoehring May 13, 2018

@lrhn Please excuse a more general remark here, as I wasn't able to find another place where this is being discussed.

Even when considering the remarks in the docs, I still think moving from SCREAMING_CAPS to lowerCamelCase is a bad design decision. The former allows for easy identification of constants against class and local variables while skimming through code (especially when it's not your own), and I'd argue this is a decade long practice for a reason in languages like Java or ActionScript (don't laugh):

_variableName //private
variableName //local (or public, while explicit getters/setters are preferred, e.g. think of breakpoints)
VARIABLE_NAME //constant

But I guess I'm like 4 years late to the party to have the conventions and APIs changed back :-)

What is super unfortunate though, is the case (sic!) of the number Pi, which dart:math introduces as a top level constant. After running the dart2_fix tool on my projects, I now have occurences of pi instead of PI everywhere, which looks like local variables one would introduce when midnight coding at a hackathon. sigh


Not entirely related, but I think renaming JSON to json is unfortunate, too.
Consider this exampe:

var json = JSON.decode(...);
var something = json["something"];
//BTW, dart2_fix will break this.

Now developers are being forced to come up with a new name for their var, e.g.

var jsonGoddammitDartTeam = json.decode(...);
var something = jsonGoddammitDartTeam["something"];

Also, isn't this change against acronym style convetions?

nilsdoehring commented May 13, 2018

@lrhn Please excuse a more general remark here, as I wasn't able to find another place where this is being discussed.

Even when considering the remarks in the docs, I still think moving from SCREAMING_CAPS to lowerCamelCase is a bad design decision. The former allows for easy identification of constants against class and local variables while skimming through code (especially when it's not your own), and I'd argue this is a decade long practice for a reason in languages like Java or ActionScript (don't laugh):

_variableName //private
variableName //local (or public, while explicit getters/setters are preferred, e.g. think of breakpoints)
VARIABLE_NAME //constant

But I guess I'm like 4 years late to the party to have the conventions and APIs changed back :-)

What is super unfortunate though, is the case (sic!) of the number Pi, which dart:math introduces as a top level constant. After running the dart2_fix tool on my projects, I now have occurences of pi instead of PI everywhere, which looks like local variables one would introduce when midnight coding at a hackathon. sigh


Not entirely related, but I think renaming JSON to json is unfortunate, too.
Consider this exampe:

var json = JSON.decode(...);
var something = json["something"];
//BTW, dart2_fix will break this.

Now developers are being forced to come up with a new name for their var, e.g.

var jsonGoddammitDartTeam = json.decode(...);
var something = jsonGoddammitDartTeam["something"];

Also, isn't this change against acronym style convetions?

@zoechi

This comment has been minimized.

Show comment
Hide comment
@zoechi

zoechi May 13, 2018

Contributor

this is a decade long practice

Isn't Dart about dropping such stuff that doesn't really carry its weight anymore?

var json = JSON.decode(...);

I'd say var json is the actual problem here. That's a poor name for the variable.
It's similar with pi. pi for the constant looks only bad if the rest of the code is poor quality like in code from

when midnight coding at a hackathon. sigh

Contributor

zoechi commented May 13, 2018

this is a decade long practice

Isn't Dart about dropping such stuff that doesn't really carry its weight anymore?

var json = JSON.decode(...);

I'd say var json is the actual problem here. That's a poor name for the variable.
It's similar with pi. pi for the constant looks only bad if the rest of the code is poor quality like in code from

when midnight coding at a hackathon. sigh

@nilsdoehring

This comment has been minimized.

Show comment
Hide comment
@nilsdoehring

nilsdoehring May 13, 2018

Didn't mean to start an argument, sorry if I sounded like that. As with most things in life, it depends on the point of view. I fully understand where you're coming from.

Where I'm coming from (digital advertising), we had a lot of freelancers coming in to create rich media experiences in no-time, and with frontend code to be thrown away after some weeks of airtime. We couldn't care less about poor code quality, so we created an environment to allow for this and still run smoothly. I semi-automatically ported all of it (>40kLOC) to Dart some time ago (keeping SCREAMING_CONSTANTS, hehe):

https://github.com/block-forest/rockdot-framework
(Background story here)

That's my personal use case.

Others might be the millions of coders who have yet to switch to Dart, especially those coming from Java (Android). And maybe people who will not switch to the language, exclusively (f.ex. I'll never forget how awesome Rails was for my use cases back then, but I had to re-learn the syntax every time I revisited the language, ultimately dropping it due to it). And who knows how many coders are out there who simply cannot (by skill or management) write good code.

Sorry for the wall of text and whataboutism. I guess I wanna stress that conventions are a balancing act with many valid viewpoints, familiarity and mediocrity being two of them ...

Thanks for your POV, thanks for a great language, and don't worry, I'll stick to Dart – just don't become Ruby/Rails ;-)

nilsdoehring commented May 13, 2018

Didn't mean to start an argument, sorry if I sounded like that. As with most things in life, it depends on the point of view. I fully understand where you're coming from.

Where I'm coming from (digital advertising), we had a lot of freelancers coming in to create rich media experiences in no-time, and with frontend code to be thrown away after some weeks of airtime. We couldn't care less about poor code quality, so we created an environment to allow for this and still run smoothly. I semi-automatically ported all of it (>40kLOC) to Dart some time ago (keeping SCREAMING_CONSTANTS, hehe):

https://github.com/block-forest/rockdot-framework
(Background story here)

That's my personal use case.

Others might be the millions of coders who have yet to switch to Dart, especially those coming from Java (Android). And maybe people who will not switch to the language, exclusively (f.ex. I'll never forget how awesome Rails was for my use cases back then, but I had to re-learn the syntax every time I revisited the language, ultimately dropping it due to it). And who knows how many coders are out there who simply cannot (by skill or management) write good code.

Sorry for the wall of text and whataboutism. I guess I wanna stress that conventions are a balancing act with many valid viewpoints, familiarity and mediocrity being two of them ...

Thanks for your POV, thanks for a great language, and don't worry, I'll stick to Dart – just don't become Ruby/Rails ;-)

@zoechi

This comment has been minimized.

Show comment
Hide comment
@zoechi

zoechi May 13, 2018

Contributor

As mentioned there was quite some discussions about this and many argued for screaming caps but the Dart team decided long ago after consideration of all arguments against using screaming caps.
The recent changes were only to make older code follow the established style guides.

Contributor

zoechi commented May 13, 2018

As mentioned there was quite some discussions about this and many argued for screaming caps but the Dart team decided long ago after consideration of all arguments against using screaming caps.
The recent changes were only to make older code follow the established style guides.

@leafpetersen

This comment has been minimized.

Show comment
Hide comment
@leafpetersen

leafpetersen May 14, 2018

Member

Not entirely related, but I think renaming JSON to json is unfortunate, too.
Consider this exampe:

var json = JSON.decode(...);
var something = json["something"];
//BTW, dart2_fix will break this.

Note that we added the jsonDecode helper to help with this scenario.

Member

leafpetersen commented May 14, 2018

Not entirely related, but I think renaming JSON to json is unfortunate, too.
Consider this exampe:

var json = JSON.decode(...);
var something = json["something"];
//BTW, dart2_fix will break this.

Note that we added the jsonDecode helper to help with this scenario.

@nilsdoehring

This comment has been minimized.

Show comment
Hide comment
@nilsdoehring

nilsdoehring May 14, 2018

@leafpetersen Thanks! Maybe the dart2_fix tool should use it when replacing references to JSON.decode. Will file an enhancement request.

@zoechi look what I found in dart2_fix's code:
Map json = jsonDecode(jsonData); ;-)

nilsdoehring commented May 14, 2018

@leafpetersen Thanks! Maybe the dart2_fix tool should use it when replacing references to JSON.decode. Will file an enhancement request.

@zoechi look what I found in dart2_fix's code:
Map json = jsonDecode(jsonData); ;-)

@terrylucas

This comment has been minimized.

Show comment
Hide comment
@terrylucas

terrylucas May 15, 2018

Member

Oops, didn't notice this issue. Leaf your recollection is correct. I've discussed this with a number of folks and the conclusion was that all JS documentation and general browser knowledge is SCREAMING_CONSTANTS. We changed some properties (along time ago) like innerHTML to innerHtml and it confused people. I guess with IDEs it easier but we want to be agnostics with frameworks we don't control.

Member

terrylucas commented May 15, 2018

Oops, didn't notice this issue. Leaf your recollection is correct. I've discussed this with a number of folks and the conclusion was that all JS documentation and general browser knowledge is SCREAMING_CONSTANTS. We changed some properties (along time ago) like innerHTML to innerHtml and it confused people. I guess with IDEs it easier but we want to be agnostics with frameworks we don't control.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment