Lecture
Это окончание невероятной информации про идеальный код.
...
( "Instantiate a Human with` new` " ) ; } Mammal . call ( this , age , furColor ) ; this . languageSpoken = languageSpoken ; } ; Human . prototype = Object . create ( Mammal . prototype ) ; Human . prototype . constructor = Human ; Human . prototype . Speak = function Speak ( ) { } ;
Good:
class Animal { constructor ( age ) { this . age = age ; } move ( ) { / * ... * / } } class Mammal extends Animal { constructor ( age , furColor ) { super ( age ) ; this . furColor = furColor ; } liveBirth ( ) { / * ... * / } } class Human extends Mammal { constructor ( age , furColor , languageSpoken ) { super ( age , furColor ) ; this . languageSpoken = languageSpoken ; } Speak ( ) { / * ... * / } }
Back to top
This pattern is very useful in JavaScript, and you see it in many libraries such as jQuery and Lodash. It lets your code be expressive and less verbose. For that reason, I say use method chaining and see how clean your code becomes. In your class functions, just return this at the end of every function, and you can chain additional class methods onto it.
Bad:
class Car { constructor ( make , model , color ) { this . make = make ; this . model = model ; this . color = color ; } setMake ( make ) { this . make = make ; } setModel ( model ) { this . model = model ; } setColor ( color ) { this . color = color ; } save ( ) { console . log ( this . make , this . model , this . color ) ; } } const car = new Car ( «Ford» , «F-150» , «red» ) ; car . setColor ( "pink" ) ; car . save ( ) ;
Good:
class Car { constructor ( make , model , color ) { this . make = make ; this . model = model ; this . color = color ; } setMake ( make ) { this . make = make ; // NOTE: Returning this for chaining return this ; } setModel ( model ) { this . model = model ; // NOTE: Returning this for chaining return this ; } setColor ( color ) { this . color = color ; // NOTE: Returning this for chaining return this ; } save ( ) { console . log ( this . make , this . model , this . color ) ; // NOTE: Returning this for chaining return this ; } } const car = new Car ( «Ford» , «F-150» , «red» ) . setColor ( "pink" ) . save ( ) ;
Back to top
As is well known from the Gang of Four's « Design Patterns », you should prefer composition over inheritance wherever possible. There are many good reasons to use inheritance and many good reasons to use composition. The gist of this maxim is that if your mind instinctively reaches for inheritance, try to think about whether composition could model your problem better. In some cases it can.
Then you might ask: «When should I use inheritance?» It depends on your problem, but here's a decent list of cases where inheritance makes more sense than composition:
Bad:
class Employee { constructor ( name , email ) { this . name = name ; this . email = email ; } // ... } // Bad, because Employees have tax data. EmployeeTaxData is not a type of Employee class EmployeeTaxData extends Employee { constructor ( ssn , salary ) { super ( ) ; this . ssn = ssn ; this . salary = salary ; } // ... }
Good:
class EmployeeTaxData { constructor ( ssn , salary ) { this . ssn = ssn ; this . salary = salary ; } // ... } class Employee { constructor ( name , email ) { this . name = name ; this . email = email ; } setTaxData ( ssn , salary ) { this . taxData = new EmployeeTaxData ( ssn , salary ) ; } // ... }
Back to top
As stated in Clean Code, «a class should have no more than one reason to change». It's tempting to pack a class with a lot of functionality, like when you can only take one suitcase with you on a flight. The problem is that your class won't be conceptually cohesive, and this gives it many reasons to change. It's important to minimize the number of times you need to change a class. This matters because if a single class contains too much functionality and you change part of it, it can be hard to understand how that will affect the other dependent modules in your codebase.
Bad:
class UserSettings { constructor ( user ) { this . user = user ; } changeSettings ( settings ) { if ( this . verifyCredentials ( ) ) { // ... } } verifyCredentials ( ) { // ... } }
Good:
class UserAuth { constructor ( user ) { this . user = user ; } verifyCredentials ( ) { // ... } } class UserSettings { constructor ( user ) { this . user = user ; this . auth = new UserAuth ( user ) ; } changeSettings ( settings ) { if ( this . auth . verifyCredentials ( ) ) { // ... } } }
Back to top
As Bertrand Meyer stated, «software entities (classes, modules, functions, etc.) should be open for extension but closed for modification». What does that mean? This principle basically states that you should allow users to add new functionality without changing existing code.
Bad:
class AjaxAdapter extends adapter { constructor ( ) { super ( ) ; this . name = "ajaxAdapter" ; } } class NodeAdapter extends adapter { constructor ( ) { super ( ) ; this . name = "nodeAdapter" ; } } class HttpRequester { constructor ( adapter ) { this . adapter = adapter ; } fetch ( url ) { if ( this . adapter . name === "ajaxAdapter" ) { return makeAjaxCall ( url ) . then ( response => { // transform the response and return } ) ; } else if ( this . adapter . name === "nodeAdapter" ) { return makeHttpCall ( url ) . then( response => { // transform the response and return } ) ; } } } function makeAjaxCall ( url ) { // request and return the promise } function makeHttpCall ( url ) { // request and return the promise }
Good:
class AjaxAdapter extends adapter { constructor ( ) { super ( ) ; this . name = "ajaxAdapter" ; } request ( url ) { // request and return the promise } } class NodeAdapter extends adapter { constructor ( ) { super ( ) ; this . name = "nodeAdapter" ; } request ( url ) { // request and return the promise } } class HttpRequester { constructor ( adapter ) { this . adapter = adapter ; } fetch ( url ) { return this . adapter . request ( URL ) . then ( response => { // transform the response and return } ) ; } }
Back to top
This is a scary term for a very simple concept. It's formally defined as «If S is a subtype of T, then objects of type T may be replaced with objects of type S (i.e., objects of type S may substitute for objects of type T) without altering any of the desired properties of that program (correctness, task performed, etc.)». That's an even scarier definition.
The best explanation is that if you have a parent class and a child class, then the base class and child class can be used interchangeably without producing incorrect results. This can still be confusing, so let's look at the classic Square-Rectangle example. Mathematically, a square is a rectangle, but if you model it with an «is-a» relationship through inheritance, you'll quickly run into problems.
Bad:
class Rectangle { constructor ( ) { this . width = 0 ; this . height = 0 ; } setColor ( color ) { // ... } render ( area ) { // ... } setWidth ( width ) { this . width = width ; } setHeight ( height ) { this . height = height ; } getArea ( ) { return this . width * this . height ; } } class Square extends Rectangle { setWidth ( width ) { this . width = width ; this . height = width ; } setHeight ( height ) { this . width = height ; this . height = height ; } } function renderLargeRectangles ( rectangles ) { rectangles . forEach ( rectangle => { rectangle . setWidth ( 4 ) ; rectangle . setHeight ( 5 ) ; const area = rectangle . getArea ( ) ; // BAD: returns 25 for Square. Should be 20. rectangle . render ( area ) ; } ) ; } const rectangles = [ new Rectangle ( ) , new Rectangle ( ) , new Square ( ) ] ; renderLargeRectangles ( rectangles ) ;
Good:
class Shape { setColor ( color ) { // ... } render ( area ) { // ... } } class Rectangle extends Shape { constructor(width, height) { super(); this.width = width; this.height = height; } getArea() { return this.width * this.height; } } class Square extends Shape { constructor(length) { super(); this.length = length; } getArea() { return this.length * this.length; } } function renderLargeShapes(shapes) { shapes.forEach(shape => { const area = shape.getArea(); shape.render(area); }); } const shapes = [new Rectangle(4, 5), new Rectangle(4, 5), new Square(5)]; renderLargeShapes(shapes);
JavaScript doesn't have interfaces, so this principle doesn't apply as strictly as the others. However, it's important and relevant even in the absence of a type system in JavaScript.
The ISP states that «clients should not be forced to depend on interfaces they do not use». Interfaces are implicit contracts in JavaScript because of duck typing.
A good example demonstrating this principle in JavaScript is classes that require large settings objects. Not requiring clients to configure a huge number of options is beneficial, because in most cases they don't need all the settings. By making them optional, you prevent a «fat interface».
Bad:
class DOMTraverser { constructor ( settings ) { this . settings = settings ; this . setup ( ) ; } setup ( ) { this . rootNode = this . settings . rootNode ; this . settings . animationModule . setup ( ) ; } traverse ( ) { // ... } } const $ = new DOMTraverser ( { rootNode : document . getElementsByTagName ( "body" ) , animationModule ( ) { } // In most cases animation is not required when moving. // ... } ) ;
Good:
class DOMTraverser { constructor ( settings ) { this . settings = settings ; this . options = settings . options ; this . setup ( ) ; } setup ( ) { this . rootNode = this . settings . rootNode ; this . setupOptions ( ) ; } setupOptions ( ) { if ( this . options . animationModule ) { // ... } } traverse ( ) { // ... } } const $ = new DOMTraverser ( { rootNode : document . getElementsByTagName ( "body" ) , options : { animationModule ( ) { } } } ) ;
Back to top
This principle talks about two important things:
At first this may be hard to understand, but if you've worked with AngularJS, you've seen an implementation of this principle in the form of dependency injection (DI). While these aren't identical concepts, DIP keeps high-level modules from knowing the details of their low-level modules and configuring them. This can be done through DI. A huge advantage of this is that it reduces the coupling between modules. Coupling is a very bad development pattern because it makes refactoring your code harder.
As noted earlier, JavaScript doesn't have interfaces, so the abstractions that depend on it are implicit contracts. That is, the methods and properties that one object/class exposes to another object/class. In the example below, the implicit contract is that any request module for the InventoryTracker object will have a requestItems method.
Bad:
class InventoryRequester { constructor ( ) { this . REQ_METHODS = [ "HTTP" ] ; } requestItem ( item ) { // ... } } class InventoryTracker { constructor ( items ) { this . items = items ; // BAD: we created a dependency on a specific request implementation. // We just need requestItems to depend on a request method: `request` this . Requester = new InventoryRequester ( ) ; } requestItems ( ) { this . items . Foreach ( item => { this . requester . requestItem ( item ) ; } ) ; } } const inventoryTracker = new InventoryTracker ( [ "apples" , "bananas" ] ) ; inventoryTracker . requestItems ( ) ;
Good:
class InventoryTracker { constructor ( items , requester ) { this . items = items ; this . requester = requester ; } requestItems ( ) { this . items . Foreach ( item => { this . requester . requestItem ( item ) ; } ) ; } } class InventoryRequesterV1 { constructor ( ) { this . REQ_METHODS = [ "HTTP" ] ; } requestItem ( item ) { // ... } } class InventoryRequesterV2 { constructor ( ) { this . REQ_METHODS = [ "WS" ] ; } requestItem ( item ) { // ... } } // By building our dependencies externally and injecting them, we can easily // swap out our request module for a new fancy one that uses WebSockets. const inventoryTracker = new InventoryTracker ( [ "apples" , "bananas" ] , new InventoryRequesterV2 ( ) ) ; inventoryTracker . requestItems ( ) ;
Back to top
Testing is more important than shipping. If you have no tests, or not enough of them, then every time you ship code you won't be sure you didn't break anything. Deciding what counts as an adequate amount is up to your team, but having 100% coverage (of all statements and branches) is how you achieve very high confidence and developer peace of mind. This means that in addition to a great testing framework, you also need to use a good coverage tool.
There's no excuse for not writing tests. There are plenty of good JS testing frameworks, so pick the one your team prefers. Once you find one that works for your team, try to always write tests for every new feature/module you introduce. If your preferred method is test-driven development (TDD), that's great, but the main thing is just to make sure you hit your coverage goals before launching any feature or refactoring an existing one.
Bad:
import assert from "assert" ; describe ( "MomentJS" , ( ) => { it ( "handles date boundaries" , ( ) => { let date ; date = new MomentJS ( "01.01.2015" ) ; date . addDays ( 30 ) ; assert . equal ( «31.01.2015» , date ) ; date = new MomentJS ( "01.02.2016" ) ; date . addDays ( 28 ) ; assert . equal ( «29.02.2016» , date ) ; date = new MomentJS ( "01.02.2015" ) ; date . addDays ( 28 ) ; assert . equal ( «01.03.2015» , date ) ; } ) ; } ) ;
Good:
import assert from "assert" ; describe ( "MomentJS" , ( ) => { it ( "handles 30-day months" , ( ) => { const date = new MomentJS ( "1/1/2015" ) ; date . addDays ( 30 ) ; assert . equal ( "31.01.2015" , date ) ; } ) ; it ( "handles a leap year" , ( ) => { const date = new MomentJS ( "2/1/2016" ) ; date . addDays ( 28 ) ; assert . equal ( "29.02.2016" , date ) ; } ) ; it ( "handles a non-leap year" , ( ) => { const date = new MomentJS ( "2/1/2015" ) ; date . addDays ( 28 ) ; assert . equal ( "03/01/2015" , date ) ; } ) ; } ) ;
Back to top
Callbacks aren't clean and they cause an excessive amount of nesting. In ES2015/ES6, promises are a built-in global type. Use them!
Bad:
import { get } from "request" ; import { writeFile } from "fs" ; get ( "https://en.wikipedia.org/wiki/Robert_Cecil_Martin" , ( requestErr , response , body ) => { if ( requestErr ) { console . error ( requestErr ) ; } else { writeFile ( "article.html" , body , writeErr => { if ( writeErr ) { console . error ( writeErr ) ; } else { console . log ( "File written" ) ; } } ) ; } } ) ;
Good:
import { get } from "request-promise" ; import { writeFile } from "fs-extra" ; get ( "https://en.wikipedia.org/wiki/Robert_Cecil_Martin" ) . then ( body => { return writeFile ( "article.html" , body ) ; } ) . then ( ( ) => { console . log ( "File written" ) ; } ) . catch ( err => { console . error ( err ) ; } ) ;
Back to top
Promises are a very clean alternative to callbacks, but ES2017/ES8 offers async and await, which provide an even cleaner solution. All you need is a function prefixed with the async keyword, and then you can write your logic imperatively without a then chain of functions. Use this if you can take advantage of ES2017/ES8 features today!
Bad:
import { get } from "request-promise" ; import { writeFile } from "fs-extra" ; get ( "https://en.wikipedia.org/wiki/Robert_Cecil_Martin" ) . then ( body => { return writeFile ( "article.html" , body ) ; } ) . then ( ( ) => { console . log ( "File written" ) ; } ) . catch ( err => { console . error ( err ) ; } ) ;
Good:
import { get } from "request-promise" ; import { writeFile } from "fs-extra" ; async function getCleanCodeArticle ( ) { try { const body = await get ( "https://en.wikipedia.org/wiki/Robert_Cecil_Martin" ) ; Await WriteFile ( "article.html" , body ) ; console . log ( "File written" ) ; } catch ( err ) { console . error ( ERR ) ; } } getCleanCodeArticle ( )
Back to top
Thrown errors are a good thing! They mean the runtime has successfully determined when something in your program went wrong, and it lets you know by stopping execution of the function on the current stack, killing the process (in Node), and notifying you in the console with a stack trace.
Doing nothing with a caught error doesn't give you the ability to fix or respond to that error. Logging the error to the console (console.log) isn't much better, since it can often get lost in the sea of things printed to the console. If you wrap any bit of code in a try/catch, it means you think an error might occur there, and so you should have a plan, or create a code path, for when it does.
Bad:
try { functionThatMightThrow ( ) ; } catch ( error ) { console . log ( error ) ; }
Good:
try { functionThatMightThrow ( ) ; } catch ( error ) { // One option (noisier than console.log): console . error ( error ) ; // Another option: notifyUserOfError ( error ) ; // Another option: reportErrorToService ( error ) ; // OR do all three! }
For the same reason, you shouldn't ignore errors caught in try/catch blocks.
Bad:
getdata ( ) . then ( data => { functionThatMightThrow ( data ) ; } ) . catch ( error => { console . log ( error ) ; } ) ;
Good:
getdata ( ) . then ( data => { functionThatMightThrow ( data ) ; } ) . catch ( error => { // One option (noisier than console.log): console . error ( error ) ; // Another option: notifyUserOfError ( error ) ; // Another option: reportErrorToService ( error ) ; // OR do all three! } ) ;
Back to top
Formatting is subjective. As with many other rules, there's no hard rule you must follow here. The main thing is NOT to ARGUE about formatting. There are tons of tools to automate this. Use one! Arguing about formatting is a waste of time and money for engineers.
For things that don't fall under automatic formatting (indentation, tabs and spaces, double or single quotes, etc.), see here for some recommendations.
JavaScript is untyped, so capitalization tells you a lot about your variables, functions, etc. These rules are subjective, so your team can choose whatever it wants. The point is that no matter what you all choose, just be consistent.
Bad:
const DAYS_IN_WEEK = 7 ; const daysInMonth = 30 ; const songs = [ "Back In Black" , "Stairway to Heaven" , "Hey Jude" ] ; const Artists = [ "ACDC" , "Led Zeppelin" , "Beatles" ] ; function eraseDatabase ( ) { } function restore_database ( ) { } class animal { } class Alpaca { }
Good:
const DAYS_IN_WEEK = 7 ; const DAYS_IN_MONTH = 30 ; const SONGS = [ «Back In Black» , «Stairway To Heaven» , «Hey Jude» ] ; const ARTISTS = [ "ACDC" , "Led Zeppelin" , "Beatles" ] ; function eraseDatabase ( ) { } function restoreDatabase ( ) { } class Animal { } class Alpaca { }
Back to top
If a function calls another, keep those functions vertically close in the source file. Ideally, keep the caller right above the callee. We tend to read code from top to bottom, like a newspaper. Because of this, make your code read that way.
Bad:
class PerformanceReview { constructor ( employee ) { this . employee = employee ; } lookupPeers ( ) { return db . lookup ( this . employee , «peers» ) ; } lookupManager ( ) { return db . lookup ( this . employee , «manager» ) ; } getPeerReviews ( ) { const peers = this . lookupPeers ( ) ; // ... } perfReview ( ) { this . getPeerReviews ( ) ; this . getManagerReview ( ) ; this . getSelfReview ( ) ; } getManagerReview ( ) { const manager = this . lookupManager ( ) ; } getSelfReview ( ) { // ... } } const review = new PerformanceReview ( employee ) ; review . perfReview ( ) ;
Good:
class PerformanceReview { constructor ( employee ) { this . employee = employee ; } perfReview ( ) { this . getPeerReviews ( ) ; this . getManagerReview ( ) ; this . getSelfReview ( ) ; } getPeerReviews ( ) { const peers = this . lookupPeers ( ) ; // ... } lookupPeers ( ) { return db . lookup ( this . employee , «peers» ) ; } getManagerReview ( ) { const manager = this . lookupManager ( ) ; } lookupManager ( ) { return db . lookup ( this . employee , «manager» ) ; } getSelfReview ( ) { // ... } } const review = new PerformanceReview ( employee ) ; review . perfReview ( ) ;
Back to top
Comments are an apology, not a requirement. Good code mostly documents itself.
Bad:
function hashIt ( data ) { // Hash let hash = 0 ; // String length const length = data . length ; // Loop over each character in the data for ( let i = 0 ; i < length ; i ++ ) { // Get the character code. const char = data . charCodeAt ( i ) ; // Build the hash hash = ( hash << 5 ) - hash + char ; // Convert to a 32-bit integer hash & = hash ; } }
Good:
function hashIt ( data ) { let hash = 0 ; length const = data . length ; for ( let i = 0 ; i < length ; i ++ ) { const char = data . charCodeAt ( i ) ; hash = ( hash << 5 ) - hash + char ; // Convert to a 32-bit integer hash & = hash ; } }
Back to top
Version control exists for a reason. Leave old code in your history.
Bad:
doStuff ( ) ; // doOtherStuff (); // doSomeMoreStuff (); // doSoMuchStuff ();
Good:
doStuff ( ) ;
Back to top
Remember, use version control! There's no need for dead code, commented-out code, and especially journal comments. Use git log to get the history!
Bad:
/ ** * 2016-12-20: Removed monads, didn't understand them (RM) * 2016-10-01: Improved using special monads (JP) * 2016-02-03: Removed type-checking (LI) * 2015-03-14: Added combine with type-checking (JR) * / function comb ( a , b ) { return a + b ; }
Good:
function comb ( a , b ) { return a + b ; }
Back to top
Usually they just add noise. Let function and variable names, along with proper indentation and formatting, give visual structure to your code.
Bad:
////////////////////////////////////////////////// ////////////////////////////// // Instantiate the Scope model /////////////// ////////////////////////////////////////////////// /////////////// $ scope . model = { menu : "foo" , nav : "bar" } ; ////////////////////////////////////////////////// ////////////////////////////// // Set up the action //////////////// ////////////////////////////////////////////////// ////////////// const actions = function ( ) { // ... } ;
Good:
$ scope . model = { menu : "foo" , nav : "bar" } ; const actions = function ( ) { // ... } ;
I hope this article was useful to you! Did I miss anything? Share your experience!
Часть 1 Perfect Code in PHP and JavaScript
Часть 2 6. The DRY principle - Perfect Code in PHP and
Часть 3 SOLID - Perfect Code in PHP and JavaScript
Comments