:bathtub: Clean Code concepts adapted for JavaScript
MIT License
79043
1827
10685

.

In Avoid Side Effects (part 2) the outbound link to facebook.github.io/immutable-js/ is broken

In most of programming languages, parameters and arguments are different concepts. Also in JS when we look at the function from inside out we say parameter and when we look at the function from outside in we say argument. And as far as I know (and read from the blogs, documentations etc), the name of the concept of defaulting the values which are passed to the function is called default parameters not default arguments So in my honest opinion this can be confusing for newbies.

It would be great to have this in Slovenian language as well.

Following the greek flag in the translation section it is written spanish and links to Spanish language page.

Unless we can find a greek page then I guess this line should be removed.

I'm not good at English but I finished reading it and found the link to the translation at the end。 GOD

Not-so-well put together brain dump follows. Note that in some parts the tone may appear a bit harsh, but I stand by my criticism and believe that some of the issues I raise below will confuse the beginner/intermediate programmers that this guide is intended to help, and at worst even do more harm than good.

🦆

Avoid Side Effects (part 1 & part 2)

https://github.com/ryanmcdermott/clean-code-javascript#avoid-side-effects-part-1
https://github.com/ryanmcdermott/clean-code-javascript#avoid-side-effects-part-2

Both of these points seem to have good advice contained within them, but I feel like they are mushing together lots of separate concepts, and could be articulated better. They are mixing together the ideas of: pure functions, side-effectful functions, centralization of side effects, global variables (evil!), global state (also evil?), and mutable vs. immutable data.

Also, I feel it is appropriate to mention here how the approach of "avoiding side effects" benefits testing.

BTW: the "good" example in part 2 calls Date.now() which is arguably a side effect.

Don't write to global functions

https://github.com/ryanmcdermott/clean-code-javascript#dont-write-to-global-functions

Very wise advice. But I don't like the example. Just write a regular function function diffArrays(array1, array2). Then you can use it with any array you get from anywhere instead of needing to craft a special type of array. Creating an Array subclass for every new function you want to perform on arrays would lead to dozens of array subclasses and madness...

Favor functional programming over imperative programming

https://github.com/ryanmcdermott/clean-code-javascript#favor-functional-programming-over-imperative-programming

Yikes.

Such a strong claim. What is "functional programming" anyway? Just like "what is OOP?" there is no single agreed-upon answer. And the authors don't explain what they mean. So the following is based upon what I personally understand "functional programming" to mean :D

JavaScript is not a good functional programming language: there is no form of "let" expression-based binding, it lacks any kind of loop fusion, there is no tail call optimization, no pattern matching, and if and switch are not expressions (although my prediction is that they might be so in a future JavaScript version).

The high order array functions (map, reduce, find, etc...) are half-baked, don't compose, and worst of all don't work with Promises nor with async/await syntax.

Let's look at the example that was given:

const programmerOutput = [
  {
    name: 'Uncle Bobby',
    linesOfCode: 500
  }, {
    name: 'Suzie Q',
    linesOfCode: 1500
  }, {
    name: 'Jimmy Gosling',
    linesOfCode: 150
  }, {
    name: 'Gracie Hopper',
    linesOfCode: 1000
  }
];

Bad:

let totalOutput = 0;

for (let i = 0; i < programmerOutput.length; i++) {
  totalOutput += programmerOutput[i].linesOfCode;
}

Good:

const totalOutput = programmerOutput
  .map(output => output.linesOfCode)
  .reduce((totalLines, lines) => totalLines + lines);

I like the "bad" code. It is just as clear, concise, readable, and understandable as the "good" code (although a "for..of" loop would be even better instead of using an index variable).

And this is a "best case" show of where "functional" programming works in JavaScript. But what if you need to do something a little more complicated? Here is a random example that I'm just pooping out of my head:

We want to find the first adjacent pair of programmers in the programmerOutput array, starting from the end, whose difference of "linesOfCode" between them is less than or equal to 1000, and return the index as well as both of their names. If no such adjacent pair of programmers exist, then return null.

The solution in "idiomatic" JavaScript using a loop is pretty simple:

function f(programmerOutput) {
    for (let i = programmerOutput.length - 1; i >= 1; i--) {
        const diff = programmerOutput[i - 1].linesOfCode - programmerOutput[i].linesOfCode;
        if (Math.abs(diff) <= 1000) {
            return {
                index: i - 1,
                name1: programmerOutput[i - 1].name,
                name2: programmerOutput[i].name
            };
        }
    }
    return null;
}

Sure, you have to be careful with your loop index and watch out for off-by-one errors, but overall pretty readable IMHO.

Now let's look at the functional solution that I was able to come up with:

function f(programmerOutput) {
    const tail = programmerOutput.slice(1);
    const pairs = tail.map((v, i) => [programmerOutput[i], v]);
    const reversed = [].concat(pairs).reverse();
    const i = reversed.findIndex(([p1, p2]) => { 
        const diff = p1.linesOfCode - p2.linesOfCode;
        return Math.abs(diff) <= 1000;
    });
    return i < 0
        ? null
        : {
            index: tail.length - i - 1,
            name1: reversed[i][0].name,
            name2: reversed[i][1].name
        };
}

Is this truly preferable? I find it a mess, and hard to understand... and I wrote it! And although performance should usually not be a concern when writing "clean code", it's worth noting that this code will be super slow and generate tons of garbage.

I'm sure there is a better way to write it functionally, especially if you use one of the "functional" JavaScript libraries; but no matter what you come up with, I still claim that nothing will be as clear as the simple "for" loop version. And this isn't a cherry-picked example: any moderately complex "functional" code you write that uses map/filter/reduce/whatever will be cleaner if it is rewritten to use plain old loops.

(BTW I'm talking only about JavaScript here... I'm sure that the functional version of this code in ClosureScript or Haskell will look beautiful)

I'm going to impudently claim that the authors of this guide don't actually believe that you should "Favor functional programming over imperative programming" when writing JavaScript. None of the examples in the rest of this guide are in any way "functional". And I doubt that the production JavaScript code that they write is functional: in their own code do they truly strongly avoid the use of any loops or "let" variables and any and all mutation?

Functional programming does have its place in JavaScript; one place where it seems to work well is with the "React" library (especially when combined with "immutablejs" library). Also, simple usages of map and filter can often indeed make code shorter and clearer. But we shouldn't go overboard and say that functional programming should be "favored" everywhere. Plain old imperative code with loops and variables is just fine, and leads to simple and clean code.

BTW: the "good" example contained in this rule written by the authors is broken. It will crash if the programmerOutput array is empty. Note that the "bad" version is well written and will return the correct result of 0. This is a good demonstration of how JavaScript's reduce is half-baked: the JavaScript committee should have made reduce's initialValue argument required. Otherwise, you end up with code like what the authors wrote here that contains the worst kind of bug: The code appears to work correctly, then you ship it to production where it continues to work fine 99% of the time until it randomly blows up on an edge case that you didn't anticipate.

Anyway, my summary is that this principle is worded much too strongly. I would re-word it to: "Use a light sprinkle of functional programming when appropriate".

Avoid conditionals

https://github.com/ryanmcdermott/clean-code-javascript#avoid-conditionals

A good rule, but I feel that not enough support is given for the advantages of this approach. I understand that this is meant to be a concise guide, so maybe add links to external references?

For example, for this rule it may be worth referring readers who are interested in a more in-depth analysis to the Anti-If Campaign website

Also later on the principles in the "SOLID" section could use some good external references.

Use getters and setters

https://github.com/ryanmcdermott/clean-code-javascript#use-getters-and-setters

hm... not sure where i stand on this one... ugly verbose boilerplate code... but I guess if your object is highly stateful then the added robustness is worth it.

btw, how do you "lazy load your object's properties" in an async language like JavaScript? I guess there are ways to do it, but an example would be cool.

Use method chaining

https://github.com/ryanmcdermott/clean-code-javascript#use-method-chaining

Not my taste personally. I find this style way too "cute" for no real tangible benefit.

Only comment things that have business logic complexity

https://github.com/ryanmcdermott/clean-code-javascript#only-comment-things-that-have-business-logic-complexity

Here is the "good" example:

function hashIt(data) {
  let hash = 0;
  const length = data.length;

  for (let i = 0; i < length; i++) {
    const char = data.charCodeAt(i);
    hash = ((hash << 5) - hash) + char;

    // Convert to 32-bit integer
    hash &= hash;
  }
}

First of all, this fails to follow the advice in Don't over-optimize (no need to cache data.length)

Second, aren't we supposed to be using functional programming? ;) This function is actually a good candidate for the use of "map" and "reduce". (I don't seriously mean this; I continue to believe that a plain old loop is the best way to write this function in JavaScript, as is done here)

BTW: both the "good" and "bad" code forget to return "hash" at the end.

But getting back to the point of this rule: in this case a named function would be better than a comment:

function to32bitInt(num) {
  return num & num;
}

function hashIt(data) {
  let hash = 0;
  for (let i = 0; i < data.length; i++) {
    const char = data.charCodeAt(i);
    hash = ((hash << 5) - hash) + char;

    hash = to32bitInt(hash);
  }
  return hash;
}

Note that a comment that WOULD be appropriate here is one describing the hash function we are using. Does it have a well-known name? What are its characteristics? Adding a comment with a link to an in-depth article explaining this hash function would also be suitable.

The End

The rest of the principles in this guide overall provide good solid advice IMHO đź‘Ť

Hi, thank for the great docs, I notice there's an absence of link in Formatting section (see HERE).

Hi @ryanmcdermott,

I replaced broken link with up-to-date French translation.

The account associated with the deposit seems to have been deleted : https://github.com/GavBaros

With your permission @ryanmcdermott , I can re-translate and replace this link to a valid repository in a PR.

hi
Could I translate this to Persian language?
i want to trans

Bumps puma from 5.5.1 to 5.6.4.

Release notes

Sourced from puma's releases.

5.6.4

  • Security
    • Close several HTTP Request Smuggling exploits (CVE-2022-24790)

The 5.6.3 release was a mistake (released the wrong branch), 5.6.4 is correct.

5.6.2 / 2022-02-11

  • Bugfix/Security
    • Response body will always be closed. (GHSA-rmj8-8hhh-gv5h, related to #2809)

5.6.1

Bugfixes

  • Reverted a commit which appeared to be causing occasional blank header values (see issue #2808) (#2809)

Full Changelog: puma/puma@v5.6.0...v5.6.1

5.6.0 - Birdie's Version

Maintainer @​nateberkopec had a daughter, nicknamed Birdie:

slack-imgs

5.6.0 / 2022-01-25

  • Features

    • Support localhost integration in ssl_bind (#2764, #2708)
    • Allow backlog parameter to be set with ssl_bind DSL (#2780)
    • Remove yaml (psych) requirement in StateFile (#2784)
    • Allow culling of oldest workers, previously was only youngest (#2773, #2794)
    • Add worker_check_interval configuration option (#2759)
    • Always send lowlevel_error response to client (#2731, #2341)
    • Support for cert_pem and key_pem with ssl_bind DSL (#2728)
  • Bugfixes

    • Keep thread names under 15 characters, prevents breakage on some OSes (#2733)
    • Fix two 'old-style-definition' compile warning (#2807, #2806)
    • Log environment correctly using option value (#2799)
    • Fix warning from Ruby master (will be 3.2.0) (#2785)
    • extconf.rb - fix openssl with old Windows builds (#2757)
    • server.rb - rescue handling (Errno::EBADF) for @notify.close (#2745)
  • Refactor

    • server.rb - refactor code using @​options[:remote_address] (#2742)
    • [jruby] a couple refactorings - avoid copy-ing bytes (#2730)

5.5.2

Re-allows UTF-8 in HTTP header values

Changelog

Sourced from puma's changelog.

5.6.4 / 2022-03-30

  • Security
    • Close several HTTP Request Smuggling exploits (CVE-2022-24790)

5.6.2 / 2022-02-11

  • Bugfix/Security
    • Response body will always be closed. (GHSA-rmj8-8hhh-gv5h, related to #2809)

5.6.1 / 2022-01-26

  • Bugfixes
    • Reverted a commit which appeared to be causing occasional blank header values (#2809)

5.6.0 / 2022-01-25

  • Features

    • Support localhost integration in ssl_bind (#2764, #2708)
    • Allow backlog parameter to be set with ssl_bind DSL (#2780)
    • Remove yaml (psych) requirement in StateFile (#2784)
    • Allow culling of oldest workers, previously was only youngest (#2773, #2794)
    • Add worker_check_interval configuration option (#2759)
    • Always send lowlevel_error response to client (#2731, #2341)
    • Support for cert_pem and key_pem with ssl_bind DSL (#2728)
  • Bugfixes

    • Keep thread names under 15 characters, prevents breakage on some OSes (#2733)
    • Fix two 'old-style-definition' compile warning (#2807, #2806)
    • Log environment correctly using option value (#2799)
    • Fix warning from Ruby master (will be 3.2.0) (#2785)
    • extconf.rb - fix openssl with old Windows builds (#2757)
    • server.rb - rescue handling (Errno::EBADF) for @notify.close (#2745)
  • Refactor

    • server.rb - refactor code using @​options[:remote_address] (#2742)
    • [jruby] a couple refactorings - avoid copy-ing bytes (#2730)

5.5.2 / 2021-10-12

  • Bugfixes
    • Allow UTF-8 in HTTP header values
Commits

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
  • @dependabot use these labels will set the current labels as the default for future PRs for this repo and language
  • @dependabot use these reviewers will set the current reviewers as the default for future PRs for this repo and language
  • @dependabot use these assignees will set the current assignees as the default for future PRs for this repo and language
  • @dependabot use this milestone will set the current milestone as the default for future PRs for this repo and language

You can disable automated security fix PRs for this repo from the Security Alerts page.

Originally posted by @dependabot in railwayapp/templates#282