no-flag-args rule for class properties declaration in constructor

[vitalyiegorov]

@Glavin001 Following class shows error:

export class MyModel {
  constructor(public isBoxAvailable = false, public isCertAvailable = false) {}
}

[Glavin001]

🤔Yes, I think this is correct and it should show an error.

The description for no-flag-args is as follows:

Functions should only do one thing, therefore passing a boolean into a function is a bad practice. The function does one thing if the flag is true and another if the flag is false!

In your case, the MyModel class does 4 different things, depending on the 2 states of both isBoxAvailable and isCertAvailable.

Consider the following:

Before

export class MyModel {
  constructor(public isBoxAvailable = false, public isCertAvailable = false) {}
}

// 4 cases:
new MyModel(false, false);
new MyModel(true, false);
new MyModel(false, true);
new MyModel(true, true);

After

// isBoxAvailable = undefined, isCertAvailable = undefined
export abstract class MyModel {
  abstract private methodUsingBoxAvailableValue();
  abstract private methodUsingCertAvailableValue();
}

// isBoxAvailable = false, isCertAvailable = false
export class MyModelWithoutBoxAvailableAndWithoutCertAvailable extends MyModel {
  private methodUsingBoxAvailableValue() {
    // do something assuming box available is false.
  }
  private methodUsingCertAvailableValue() {
    // do something assuming box available is false.
  }
}

// isBoxAvailable = true, isCertAvailable = false
export class MyModelWithBoxAvailableAndWithoutCertAvailable extends MyModel {
  private methodUsingBoxAvailableValue() {
    // do something assuming box available is true.
  }
  private methodUsingCertAvailableValue() {
    // do something assuming box available is false.
  }
}

// isBoxAvailable = false, isCertAvailable = true
export class MyModelWithoutBoxAvailableAndWithCertAvailable extends MyModel {
  private methodUsingBoxAvailableValue() {
    // do something assuming box available is false.
  }
  private methodUsingCertAvailableValue() {
    // do something assuming box available is true.
  }
}

// isBoxAvailable = true, isCertAvailable = true
export class MyModelWithBoxAvailableAndCertAvailable extends MyModel {
  private methodUsingBoxAvailableValue() {
    // do something assuming box available is true.
  }
  private methodUsingCertAvailableValue() {
    // do something assuming box available is true.
  }
}

// 4 cases:
new MyModelWithoutBoxAvailableAndWithoutCertAvailable(); // === new MyModel(false, false);
new MyModelWithBoxAvailableAndWithoutCertAvailable(); // === new MyModel(true, false);
new MyModelWithoutBoxAvailableAndWithCertAvailable(); // === new MyModel(false, true);
new MyModelWithBoxAvailableAndCertAvailable(); // === new MyModel(true, true);

See that the above classes are specific to each of their 4 specific use cases. They do exactly 1 thing.

Of course, the above code is contrived and likely does not make the most sense in your particular case. However, the general idea is removing boolean values such that if (this.isBoxAvailable) and if (this.isCertAvailable) lines can be removed throughout the class. It should be implemented via the separated classes.

Hope that helps!

[Glavin001]

See https://github.com/Glavin001/tslint-clean-code/blob/master/src/tests/NoFlagArgsRuleTests.ts for currently passing tests. Feel free to submit a Pull Request if you think there is a bug to be fixed.