Node: Convert classes to types #2005
Conversation
GumpacG
force-pushed
the
node/integ_guiang_classes_to_types
branch
from
350bc30
to
73a709f
Compare
|
|
||
| if (options) { | ||
| args = args.concat(options.toArgs()); | ||
| if (options.rank !== undefined) { |
There was a problem hiding this comment.
Can't you just use if (options.rank) without checking explicitly for undefined? Same for the other usages here.
There was a problem hiding this comment.
We can do that with non-number types however, we can't do that with number types because it can be 0 which would be treated as false and behaves differently.
There was a problem hiding this comment.
Be careful. The string value "" is also considered falsey - not relevant here, but could be relevant in certain circumstances.
https://www.freecodecamp.org/news/falsy-values-in-javascript/
|
|
||
| if (options) { | ||
| args = args.concat(options.toArgs()); | ||
| if (options.rank !== undefined) { |
There was a problem hiding this comment.
Be careful. The string value "" is also considered falsey - not relevant here, but could be relevant in certain circumstances.
https://www.freecodecamp.org/news/falsy-values-in-javascript/
| } | ||
|
|
||
| if (options.changed) { | ||
| args.push("CH"); |
There was a problem hiding this comment.
do we have a place to put string constants?
There was a problem hiding this comment.
Not currently, I think if we want to do this, it should be a part of a different PR as we may want to convert other types with string constants to enums. Would potentially be big enough on it's own.
GumpacG
force-pushed
the
node/integ_guiang_classes_to_types
branch
2 times, most recently
from
0bc249b
to
8b623af
Compare
Signed-off-by: Guian Gumpac <[email protected]>
Signed-off-by: Guian Gumpac <[email protected]>
GumpacG
force-pushed
the
node/integ_guiang_classes_to_types
branch
from
8b623af
to
a881ab2
Compare
Signed-off-by: Guian Gumpac <[email protected]>
Description
Converted classes to types and added them to Command.ts to follow good TypeScript practice.
Converted:
These changes affects the commands: