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

Improves readability of code examples #224

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tomasbonco
Copy link

Signals themselves are quite complex. Aim of this small PR is to slightly improve readability of code samples. I hope especially beginners and devs not familiar with the topic appreciate it.

  • When comparing, using === instead of == is just a good practice. Additionally, stops reader from questions "Why they didn't use ===? Can this be null or undefined? Do I need to read docs?".
  • Changing let to const separates things that change from those that don't change, so reader don't have to keep in mind all variables.
  • Adding extra brackets to for emphasis there is a code on the same line and reader should not jump directly to the next line.

Improves readability especially for beginners.
* When comparing, using `===` instead of `==` is just a good practice. Additionally, stops reader from questions "Can this be null or undefined? Do I need to read docs?".
* Changing `let` to `const` separates things that change from those that don't change, so reader don't have to keep in mind all variables.
* Adding extra brackets to `for` emphasis there is a code on the same line and reader should not jump directly to the next line.
README.md Outdated Show resolved Hide resolved
@@ -370,7 +370,7 @@ let w = new Signal.subtle.Watcher(() => {
// itself on the microtask queue whenever one of its dependencies might change
export function effect(cb) {
let destructor;
let c = new Signal.Computed(() => { destructor?.(); destructor = cb(); });
const c = new Signal.Computed(() => { destructor?.(); destructor = cb(); });

Choose a reason for hiding this comment

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

Same should we go multiline here ?

Copy link
Author

@tomasbonco tomasbonco May 19, 2024

Choose a reason for hiding this comment

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

Well, I know this is sort of personal taste, but in my opinion this is acceptable. Lambda functions are often one-liners and the content in this case is not too complex. I assume a reader is used to reading the rest of the line. Understanding the purpose of the code is going to be the issue, but not its style.

@tomasbonco tomasbonco requested a review from JeanMeche May 19, 2024 22:03
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