Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link
Community Bot
Community Bot
  • 1

Very clean Dart code! Just a couple of (minor) styling issues issues I can see:

Screaming caps

In the Effective Dart: Style guide we are told to avoid screaming caps for constants unless we are extending older Dart code (it was originally Dart style to use screaming caps).

From the documentation linked to above:

Note: We initially used Java’s SCREAMING_CAPS style for constants. We changed for a few reasons:

 
  • SCREAMING_CAPS looks bad for many cases, particularly enum values for things like CSS colors.
  • Constants are often changed to final non-const variables, which would necessitate a name change.
  • The values property automatically defined on an enum type is const and lowercase.

... so you might want to rename SQUARE_SIZE to squareSize.

If there are many such constants it might help to package them in their own file (e.g. import 'globals.dart' as globals;) or an abstract class (see avoid static-member-only classes where the guide gives exceptions to the rule).

KeyboardEvent.key

This is more of a DOM than Dart issue, and may not be what you want, but it might improve your code readability to use the KeyboardEvent.key property rather than KeyboardEvent.keyCode. (See more at the MDN docs.) In that case directions could be a more easily read Map<String, Function> object. Also:

This feature [keyCode] is no longer recommended. Though some browsers might still support it, it may have already been removed from the relevant web standards, may be in the process of being dropped, or may only be kept for compatibility purposes... MDN source

Misc.

Also, if directions is only going to be defined once and not changed during execution, consider making it final and defining it where where you declared it. (I can't find the docs for this point but I vaguely remember reading something along these lines somewhere.)

Very clean Dart code! Just a couple of (minor) styling issues issues I can see:

Screaming caps

In the Effective Dart: Style guide we are told to avoid screaming caps for constants unless we are extending older Dart code (it was originally Dart style to use screaming caps).

From the documentation linked to above:

Note: We initially used Java’s SCREAMING_CAPS style for constants. We changed for a few reasons:

 
  • SCREAMING_CAPS looks bad for many cases, particularly enum values for things like CSS colors.
  • Constants are often changed to final non-const variables, which would necessitate a name change.
  • The values property automatically defined on an enum type is const and lowercase.

... so you might want to rename SQUARE_SIZE to squareSize.

If there are many such constants it might help to package them in their own file (e.g. import 'globals.dart' as globals;) or an abstract class (see avoid static-member-only classes where the guide gives exceptions to the rule).

KeyboardEvent.key

This is more of a DOM than Dart issue, and may not be what you want, but it might improve your code readability to use the KeyboardEvent.key property rather than KeyboardEvent.keyCode. (See more at the MDN docs.) In that case directions could be a more easily read Map<String, Function> object. Also:

This feature [keyCode] is no longer recommended. Though some browsers might still support it, it may have already been removed from the relevant web standards, may be in the process of being dropped, or may only be kept for compatibility purposes... MDN source

Misc.

Also, if directions is only going to be defined once and not changed during execution, consider making it final and defining it where where you declared it. (I can't find the docs for this point but I vaguely remember reading something along these lines somewhere.)

Very clean Dart code! Just a couple of (minor) styling issues issues I can see:

Screaming caps

In the Effective Dart: Style guide we are told to avoid screaming caps for constants unless we are extending older Dart code (it was originally Dart style to use screaming caps).

From the documentation linked to above:

Note: We initially used Java’s SCREAMING_CAPS style for constants. We changed for a few reasons:

  • SCREAMING_CAPS looks bad for many cases, particularly enum values for things like CSS colors.
  • Constants are often changed to final non-const variables, which would necessitate a name change.
  • The values property automatically defined on an enum type is const and lowercase.

... so you might want to rename SQUARE_SIZE to squareSize.

If there are many such constants it might help to package them in their own file (e.g. import 'globals.dart' as globals;) or an abstract class (see avoid static-member-only classes where the guide gives exceptions to the rule).

KeyboardEvent.key

This is more of a DOM than Dart issue, and may not be what you want, but it might improve your code readability to use the KeyboardEvent.key property rather than KeyboardEvent.keyCode. (See more at the MDN docs.) In that case directions could be a more easily read Map<String, Function> object. Also:

This feature [keyCode] is no longer recommended. Though some browsers might still support it, it may have already been removed from the relevant web standards, may be in the process of being dropped, or may only be kept for compatibility purposes... MDN source

Misc.

Also, if directions is only going to be defined once and not changed during execution, consider making it final and defining it where where you declared it. (I can't find the docs for this point but I vaguely remember reading something along these lines somewhere.)

Source Link
Richard Ambler
Richard Ambler
  • 141
  • 6

Very clean Dart code! Just a couple of (minor) styling issues issues I can see:

Screaming caps

In the Effective Dart: Style guide we are told to avoid screaming caps for constants unless we are extending older Dart code (it was originally Dart style to use screaming caps).

From the documentation linked to above:

Note: We initially used Java’s SCREAMING_CAPS style for constants. We changed for a few reasons:

  • SCREAMING_CAPS looks bad for many cases, particularly enum values for things like CSS colors.
  • Constants are often changed to final non-const variables, which would necessitate a name change.
  • The values property automatically defined on an enum type is const and lowercase.

... so you might want to rename SQUARE_SIZE to squareSize.

If there are many such constants it might help to package them in their own file (e.g. import 'globals.dart' as globals;) or an abstract class (see avoid static-member-only classes where the guide gives exceptions to the rule).

KeyboardEvent.key

This is more of a DOM than Dart issue, and may not be what you want, but it might improve your code readability to use the KeyboardEvent.key property rather than KeyboardEvent.keyCode. (See more at the MDN docs.) In that case directions could be a more easily read Map<String, Function> object. Also:

This feature [keyCode] is no longer recommended. Though some browsers might still support it, it may have already been removed from the relevant web standards, may be in the process of being dropped, or may only be kept for compatibility purposes... MDN source

Misc.

Also, if directions is only going to be defined once and not changed during execution, consider making it final and defining it where where you declared it. (I can't find the docs for this point but I vaguely remember reading something along these lines somewhere.)