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 <https://francescocirillo.com/pages/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 <https://github.com/ryanmcdermott/clean-code-javascript#dont-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 👍 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#242>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AX3CKdJENuv1GVn0c8-1HueInJWUgX92ks5tuZw-gaJpZM4TvT1E> .