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

Add note on concatenating a string to an existing value #320

Closed
wants to merge 1 commit into from

Conversation

heff
Copy link

@heff heff commented Apr 22, 2015

We're in the process of converting Video.js to ES6, and one of our devs assumed that this meant all string concatenation should be replaced with string templates. In some cases though this makes the code less readable. It's more obvious when you're using more complicated variables.

const rateName = `${this.player.getPlaybackRate()}x`
// vs
const rateName = this.player.getPlaybackRate() + 'x'

The general rule we landed on was:

  • When variables are being added to a string, use a string template
  • When a string is being added to a variable, use concatenation

There could be a better way to say that. For the original example, see videojs/video.js#2015 (comment)

We're in the process of converting Video.js to ES6, and one of our devs assumed that this meant all string concatenation should be replaced with string templates. In some cases though this makes the code less readable. It's more obvious when you're using more complicated variables.

```
const rateName = `${this.player.getPlaybackRate()}x`
// vs
const rateName = this.player.getPlaybackRate() + 'x'
```

The general rule we landed on was:
- When variables are being added to a string, use a string template
- When a string is being added to a variable, use concatenation

There could be a better way to say that. For the original example, see videojs/video.js#2015 (comment)
@nkbt
Copy link

nkbt commented Apr 22, 2015

Imho the rule should be super simple. If you need variables in strings, always use templates. Use strings only for plain strings. And don't use concatenation (unless it is essential for particular case and commented).
At least this is what we use in our team.

@nkbt
Copy link

nkbt commented Apr 22, 2015

If you add some code highlight, then there is no issue spotting a static suffix/prefix at all.

const rateName = `${this.player.getPlaybackRate()}x`
// vs
const rateName = this.player.getPlaybackRate() + 'x'

Or screen from WebStorm

20150422-113516

If you use Sublime, with default color scheme for JS that might seem like a problem, but should be solvable. After all it is not a problem with code style, but with editor.

20150422-113701

@goatslacker
Copy link
Collaborator

Feel free to fork the the guide for your own organization and add your own rules. You're welcome to submit a PR to the in the wild section with a link to your guide.

As for this rule, we don't follow it at Airbnb. We feel it's ok with syntax highlighting.

@heff
Copy link
Author

heff commented Apr 22, 2015

No worries, thanks guys!

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.

3 participants