ryanmcdermott / clean-code-javascript

:bathtub: Clean Code concepts adapted for JavaScript
MIT License
91.85k stars 12.33k forks source link

SRP example violates dependency inversion principle #214

Open baronswindle opened 7 years ago

baronswindle commented 7 years ago

Note: This is probably a bit of a nit, and others might not share my opinion.

Here's the current "Good" example illustrating the SRP:

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()) {
      // ...
    }
  }
}

As you can see, the constructor for the UserSettings class depends on the concrete class UserAuth. This violates the dependency inversion principle. This might be an overly ambitious goal, but I think it would be helpful to the reader if we were consistent about what constitutes clean code and didn't provide examples that explicitly violate other principles. To that end, I suggest we rewrite this example to look something like this:

class UserAuth {
  constructor(user) {
    this.user = user;
  }

  verifyCredentials() {
    // ...
  }
}

class UserSettings {
  constructor(user, auth) {
    this.user = user;
    this.auth = auth;
  }

  changeSettings(settings) {
    if (this.auth.verifyCredentials()) {
      // ...
    }
  }
}

If others agree, I can make this PR.

ryanmcdermott commented 7 years ago

Thanks you're right! Can you open a PR?