HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Which one is more readable for mutually exclusive but mandatory command line parameters validation?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
readablemutuallymandatorylineexclusivebutmorevalidationonefor

Problem

I have a small validation code that needs to make sure that exactly one of two parameter is
set

I consider two options, the "Naive" that has some repetitions and might look "clumsy" but is very clear, and a second that is a bit more "DRY" but I'm not sure it's as readable / convincing in it's correctness.

Option one

if(param1==null && param2==null){
  throw new RuntimeException("Either param1 or param2 must be specified");
}

if(param1!=null && param2!=null){
  throw new RuntimeException("Only one of param1 or param2 must be specified");
}

if(param1!=null){
  //do something with param1
}
if(param2!=null){
  //do something with param2
}


Option two

if(param1!=null){
  if(param2!=null){
    //only runs if both are not null
    throw new RuntimeException("Only one of param1 or param2 must be specified");
  }
  //do something with param1
} else if(param2!=null){
  //do something with param2
} else{
  //only runs if both are null
  throw new RuntimeException("Either param1 or param2 must be specified");
}


Which one is more readable? which one would you keep? do you see any issue with Option 2?

Edit:

Based on feedback, I think there is a bit nicer option

Option three

boolean isParam1Valid = param1!=null;
boolean isParam2Valid = param2!=null;

if(isParam1Valid == isParam2Valid ){ //e.g. either both are null or both are not null
  throw new RuntimeException("param1 or param2 must be specified exactly once");
}

if(isParam1Valid){
  //do something with param1
}

if(isParam2Valid){
  //do something with param2
}


Option four shorter, but a bit less readable IMHO than option three (one needs to go up and read the code to understand why author is so sure that the else block means that param2 is valid

```
boolean isParam1Valid = param1!=null;
boolean isParam2Valid = param2!=null;

if(isParam1Valid == isParam2Valid ){ //e.g. either both are null or both are not null
throw new RuntimeException("param1 or param2 must be specified exactly once"

Solution

I would put all validations first, and then extract to a method. Better, trying using a parsing framework like JCommander that might be able to handle that for you, so that your code is just business logic

Context

StackExchange Code Review Q#19271, answer score: 3

Revisions (0)

No revisions yet.