if (!contains(username, ' ')) {
if (!temp.equals(username)) {
if (!map.containsKey(username)) {
if (map.containsKey(temp)) {
if (map.get(temp).equals(password))
map.put(username, map.remove(temp));
else
System.out.println("The password is incorrect.");
} else {
System.out.println("The username is incorrect.");
}
} else {
System.out.println("This username already exists.");
}
} else {
System.out.println("The usernames cannot be the same.");
}
} else {
System.out.println("Username cannot contain a space.");
}
Here is a copied sample from my program. As you can see there is a lot of indentation. Indentation that makes the code harder to read. Is there anything I can do to make the code more readable? Thanks.
CodePudding user response:
Invert the validation conditions and use else if
:
if (contains(username, ' ')) {
System.out.println("Username cannot contain a space.");
} else if (temp.equals(username)) {
System.out.println("The usernames cannot be the same.");
} else if (map.containsKey(username)) {
System.out.println("This username already exists.");
} else if (!map.containsKey(temp)) {
System.out.println("The username is incorrect.");
} else if (!map.get(temp).equals(password)) {
System.out.println("The password is incorrect.");
} else { // everything's valid
map.put(username, map.remove(temp));
}
However, you might want not to fail fast but collect as many validation errors as possible, so a list of errors can be prepared without else-if
and then checked if it's empty:
List<String> errors = new ArrayList<>();
if (contains(username, ' ')) {
errors.add("Username cannot contain a space.");
}
if (temp.equals(username)) {
errors.add("The usernames cannot be the same.");
}
if (map.containsKey(username)) {
errors.add("This username already exists.");
}
if (!map.containsKey(temp)) {
errors.add("The username is incorrect.");
}
if (!map.get(temp).equals(password)) {
errors.add("The password is incorrect.");
}
if (errors.isEmpty()) { // everything's valid
map.put(username, map.remove(temp));
} else {
errors.forEach(System.out::println);
}
CodePudding user response:
There are a few approaches that would work here, but IMO the simplest is the builder pattern. I would start by extracting the various tests into primitive boolean
(s) and use a StringJoiner
as the message builder. Like,
boolean containsSpace = username.contains(" ");
boolean userExists = !temp.equals(username);
boolean userNotInMap = !map.containsKey(username);
boolean userCorrect = userNotInMap && map.containsKey(temp);
boolean passCorrect = userCorrect && map.get(temp).equals(password);
StringJoiner sj = new StringJoiner(System.lineSeparator());
if (containsSpace) {
sj.add("Username cannot contain a space.");
}
if (!userNotInMap) {
sj.add("The usernames cannot be the same.");
}
if (!userExists) {
sj.add("This username already exists.");
}
if (!userCorrect) {
sj.add("The username is incorrect.");
}
if (!passCorrect) {
sj.add("The password is incorrect.");
}
if (sj.length() == 0) {
map.put(username, map.remove(temp));
} else {
System.out.println(sj);
}
CodePudding user response:
For each of the five outputs ("The password is incorrect.", etc), construct the the five input (!contains(username, ' '),etc.) truth tables. From these, construct the Karnaugh-maps. From these, construct the Sum of Products or the Product of Sums boolean equations. From these, write the code (in the form if((ABC...) (ABC...)...) or (A B C)(D E F)..., where A,B,C... are contains(username, ' '), !contains(username, ' ')temp.equals(username),...) using only 1 level of indentation.
This method can also catch and resolve races.
Not all languages guarantee the required ordering for this to work. Depending on your situation, some variable values may not be valid depending on the values of other variables. In this case you will need to consider the order. For example, if B is bogus when A is false, then if(A and B) is ok (if A is false, B is not checked), but if(B and A) could fail even though logically the two are equivalent.
The ordering of the resultant, "one-indent", ifs, might also need to be carefully chosen, depending on your situation.