Skip to content

Conversation

gbrandtio
Copy link
Contributor

@gbrandtio gbrandtio commented Jan 24, 2023

Terminology for magic literals might be beneficial, in a sense that will make a strong impression to people and will be remembered when writing/reviewing code.

Also allows easier communication between peers, since the terminology used is an industry standard (everybody will know what the discussion is about, when it comes to magic literals, magic numbers, magic strings).

@gbrandtio
Copy link
Contributor Author

@dimadeveatii could you kindly review this 😄 ?

README.md Outdated

```ts
// Declare them as capitalized named constants.
// Declare them as capitalized symbolic constants.
Copy link
Member

@dimadeveatii dimadeveatii Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is symbolic constants better than named constants? The use of term symbolic might confuse people to use Symbol, which is not the intention.

Copy link
Contributor Author

@gbrandtio gbrandtio Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The term symbolic constants is widely used throughout SE. However, seeing your next comment I do understand that the intention is to have a more straight-forward way for beginners to better get up to speed.

Is there any way we can incorporate both ? Something like the below for instance (might also help someone do further research on the term / get more familiar with):

// Declare them as capitalized symbolic constants (also known as symbolic constants).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The term symbolic constant is more oftenly used in C/C++, you would rarely see it in a JS/TS reference. It is also mentioned as literal constant in other places. I prefer to keep it simple and not confuse the reader with too much terminology.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed on 36d14bf

README.md Outdated
Comment on lines 120 to 122
### Use searchable names / Avoid Magic Literals

We will read more code than we will ever write. It's important that the code we do write must be readable and searchable. By *not* naming variables that end up being meaningful for understanding our program, we hurt our readers. Make your names searchable. Tools like [ESLint](https://typescript-eslint.io/) can help identify unnamed constants.
We will read more code than we will ever write. It's important that the code we do write must be readable and searchable. By *not* naming variables that end up being meaningful for understanding our program, we hurt our readers. Make your names searchable. Tools like [ESLint](https://typescript-eslint.io/) can help identify magic literals.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The term searchable names is just what it says and it's easy to understand for a beginner who's not yet comfortable with all of the programming dictionary. While you say magic literals, others call them magic strings.

I consider the term searchable names less confusing and easier to understand for someone who is a beginner.
I do appreciate your contribution, and I'd suggest that we add it to the end of the paragraph instead:

...can help identify unnamed constants (also known as magic literals and magic strings).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And in the end, the name searchable names is referenced in the original Clean Code book (see Chapter 2, page 22 - Use Searchable Names).

Copy link
Contributor Author

@gbrandtio gbrandtio Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - the terminology for magic literals referes to both magic numbers and magic strings. Indeed though someone might be overwhelmed by this terminology.

Addressed this as per the above comments, explicitly mentioning magic numbers and magic strings and bringing back the unnamed constants on d1e2cfb

Copy link
Member

@dimadeveatii dimadeveatii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one 👍 . Please see my comments above.

- Removed "/Avoid magic literals" from the header to avoid noise.
- Added back unnamed constants and kept magic strings, magic numbers as a note at the end of the paragraph.
@dimadeveatii dimadeveatii merged commit 9e492d2 into labs42io:main Feb 15, 2023
mheob added a commit to mheob/clean-code-typescript that referenced this pull request Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants