How can I make my if statements less contradictory?-CodePudding

I have a points system for a password strength checker that I am working on.

Currently the rules work like something this: 5 if the user entry has a lower case letter, 10 if it has a lower and upper case letter, a number or a special character ,etc.

The problem I have ran into is that my if statements are contradictory and I was wandering if there were any alternatives I could use.

The statements below is me trying to get it so if they put in a certain type of character, they will gain points and it will contribute to the score, i.e. if it has a number, then 5 to the score.

 if (userPassword.Any(char.IsDigit))
        {
            points  = 5;
        }

        if (userPassword.Any(char.IsLower))
        {
            points  = 5;
        }

        if (userPassword.Any(char.IsUpper))
        {
            points  = 5;
        }

        if (userPassword.Any(c => specialCharacters.Contains(c)))
        {
            points  = 5;
        }

To get rid of the contradiction, I tried to do an .all function on the statements that potentially deduct points of the user, but it seems inadequate and I don't think it is quite right for this programme.

I am basically trying to get it to deduct points if the user is using only one type of character, i.e. if they only use numbers and nothing else they will lose points.

  if (userPassword.All(char.IsLower) || userPassword.All(char.IsUpper)) 
        {
            points -= 5;
        }

        if (userPassword.All(char.IsDigit)) 
        {
            points -= 5;
        }

        if (userPassword.All(c => specialCharacters.Contains(c))) 
        {
            points -= 5;
        }
    }

What would be an alternative to solve this?

CodePudding user response：

If I'm understanding correctly, you want passwords which fit the "all uppercase," et cetera conditionals to subtract points while also NOT adding points for having a single uppercase. Like currently, "abc" in your example would receive 5 for having one lowercase, then would lose 5 for having all lowercase, evening out to 0 rather than -5.

I would put your point-deducting conditionals into a single if statement at the beginning, then put all of your point-adding conditionals into if statements within an else statement:

if (userPassword.All(char.IsLower) ||
  userPassword.All(char.IsUpper) ||
  userPassword.All(char.IsDigit) ||
  userPassword.All(c => specialCharacters.Contains(c)) {
    points -= 5;
  } else {
      if (userPassword.Any(char.IsDigit)) {
        points  = 5;
      }
      if (userPassword.Any(char.IsLower)) {
        points  = 5;
      }
      if (userPassword.Any(char.IsUpper)) {
        points  = 5;
      }
      if (userPassword.Any(c => specialCharacters.Contains(c))) {
        points  = 5;
      }
    }

Alternatively, if you needed those point-deducting conditionals to be separated to allow for different values of point deductions for different conditions, just use more else if statements for the bad ones:

if (userPassword.All(char.IsLower) || userPassword.All(char.IsUpper)) {
  points -= 5;
} else if (userPassword.All(char.IsDigit)) {
  points -= 5;
} else if (userPassword.All(c => specialCharacters.Contains(c))) {
  points -= 5;
} else {
  if (userPassword.Any(char.IsDigit)) {
    points  = 5;
  }
  if (userPassword.Any(char.IsLower)) {
    points  = 5;
  }
  if (userPassword.Any(char.IsUpper)) {
    points  = 5;
  }
  if (userPassword.Any(c => specialCharacters.Contains(c))) {
    points  = 5;
  }
}

CodePudding user response：

Combine your "contradiction" check with the relevant "affirmation" check. For example, when testing for containing a digit, also ensure they're not all digits:

if( userPassword.Any(char.isDigit)
    && !userPassword.All(char.isDigit) )
{
    points  = 5;
}
else /*if(userPassword.All(char.isDigit)*/ // if desired
{
    points -= 5; // if appropriate
}

Now lets discuss restructuring your algorithm. Instead of defining long chains of if or if/else statements, let's create a class you can use to define your checks:

public class PasswordEvaluator
{
    public Func<string, bool> TestCondition { get; init set; }
    public int AffirmativeResultValue { get; init set; }
    public int NegativeResultValue { get; init set; }

    public int Evaluate(string password) =>
        TestCondition(password)
            ? AffirmativeResultValue 
            : NegativeResultValue ;
}

Then you can define your tests. Using the above solution:

var hasDigitEvaluator = new PasswordEvaluator
{
    Condition = (string password) => password.Any(char.isDigit)
        && !password.All(char.isDigit),
    AffirmativeResultValue = 5,
    NegativeResultValue = -5, // or 0, whatever
};

Then you can use this object to perform the operation:

points  = hasDigitEvaluator.Evalutate(userPassword);

Now, to handle multiple evaluators, you can compose an IEnumerable<PasswordEvaluator> and then call a single operation over it to get your aggregate result:

var passwordEvaluators = new[]
{
    new PasswordEvaluator { ... },
    new PasswordEvaluator { ... },
    ...
};

var points = passwordEvaluators
    .Select(test => test.Evaluate(userPassword))
    .Sum();