Improve if-statement for multiple strings

Question

I working with a lot of Strings which I get from a web-form:

final String user = request.getParameter("user");
final String pw = request.getParameter("password");
final String name = request.getParameter("name");
final String shortname = request.getParameter("shortname");
final String appl_name = request.getParameter("appl_name");
final String db_size_str = request.getParameter("db_size");
final String was_admin_string = request.getParameter("was_admin");
final String sap_nr = request.getParameter("sap_nr");
final String etl_string = request.getParameter("etl");
final String mq_string = request.getParameter("mq");
final String msg_broker_string = request.getParameter("msgbroker");
final String java_batch = request.getParameter("javab");
final String server = request.getParameter("server");
final String apverantw1 = request.getParameter("appliverantw1");
final String apverantw2 = request.getParameter("appliverantw2");
final String psp = request.getParameter("psp");
final String version = request.getParameter("version");
final String applialias = request.getParameter("applialias");
final String applityp = request.getParameter("applityp");
final String data = request.getParameter("data");
final String yesdata = request.getParameter("yesdata");
final String moreoptions = request.getParameter("moreoptions");

If only one of them is empty, the program should be stopped. At this time I check that with an ugly if-statement if no one of the strings is empty:

if (user != null && name != null && shortname != null && appl_name != null && db_size_str != null && was_admin_string != null && sap_nr != null && etl_string != null && mq_string != null
                && msg_broker_string != null && java_batch != null && server != null && apverantw1 != null && apverantw2 != null && psp != null && version != null && applialias != null
                && applityp != null && data != null && yesdata != null && moreoptions != null)

This was produced over the time when I added one string after one.
How can I improve this? I mean, it is a really big, ugly if-statement. But I don't have a clue how can I write this even better.

Answer 1

i assume that you don't want to proceed the request, and send error message about missing string to the user, and also you are not using any MVC framework.

First you need to define a list of parameter names. Then define the below method,

 List<String> paramNames =  ... // you prepared a list of parameter names

List<String> validate(HttpServletRequest req ) {
   List<String> emptyParams = new ArrayList<>();

   for(String name : paramNames ){
      if(StringUtils.isBlank( req.getParameter(name))){
          emptyParams.add(param);
       }
   }

   return emptyParams;  
 }

Note: StringUtils class comes from the apache common library.

now the above method can tell you what parameters are empty. you can decide what to do based on the result.

if you use any mvc framework, it is better to use form binding logic and validation provided by the framework.

Answer 2

You can build a method which pulls a String from request and check if it is empty:

private static String getString(Request request, String parameter){

    String result = request.getParameter(parameter);
    if(result==null||result.isEmpty()){
        System.exit(0);
    }
    return result;
}

Answer 3

You may (beside PKopachevsky's suggestion) as well create a new class for this with Request request being a member, and a getParameter method with the same signature as the one of Request, so you spare repeating the same request parameter again and again:

class RequestAdapter {

private Request request;
public RequestAdapter(Request request) {
    this.request = request;
}
public String getParameter(String parameterName) {
    String result = request.getParameter(parameterName);
    if (result==null) {
        System.exit(0);
    }
    return result;
}

Your may use it like this:

RequestAdapter request = new RequestAdapter(originalRequest);
final String user = request.getParameter("user");
final String pw = request.getParameter("password");
final String name = request.getParameter("name");
// and so on...

Answer 4

nvseenu's answer is good but I like varargs. In your getParameter method

{
    String user = .... // all the parameters
    .
    .
    .
    // then
    validate(user, pwd, ...) // and all this may look big but with auto-completion 
                             // it's not a big headache
}

private void validate(String... parameters) {
    for(String parameter : parameters) {
        if(parameter == null || "".equals(parameter)) {
            System.exit(0); // but it's bad IMO
        }
    }
    return;
}

Answer 5

Let me assume your class name is SomeRequestBuilder. With the following solution I will try to validate and get the request value of a specific parameter at the same time. So you do not have to fetch same value twice - one for validation and other one to process the value. I have used reflection to do this. I see you have declared all parameters as final, so I had to use setAccessible as true. In case you do not need those as final then setAccessible can be removed. This code can be improved further with better exception handing. Here I wanted to show you how to do validation with less code.

class SomeRequestBuilder { 
  private final String user = "";
  private final String password = "";                                                                                                                                                                                                                                                   
  private final String name = "";
  private final String shortname = "";                                                                                                                                                                                                                                                  
  private final String applName = ""; 
  private final String dbSize = "";                                                                                                                                                                                                                                                    
  private final String wasAdmin = "";
  private final String sapNr = "";                                                                                                                                                                                                                                                     
  private final String etl = "";                                                                                                                                                                                                                                                        
  private final String mq = "";
  private final String msgbroker = "";                                                                                                                                                                                                                           
  private final String javab = ""; 
  private final String server = ""; 
  private final String appliverantw1 = ""; 
  private final String appliverantw2 = "";                                                                                                                                                                                                                                              
  private final String psp = "";  
  private final String version = ""; 
  private final String applialias = ""; 
  private final String applityp = "";
  private final String data = ""; 
  private final String yesdata = ""; 
  private final String moreoptions = "";  

  public void validateAndSetValues(SomeRequest request) {
     String input = "";  
     Field[] fields = getClass().getDeclaredFields();
     for (Field field : fields) { 
       try {
         input = request.getParameter(field.getName()); 
         if (null == input) { 
           // return here...
         } 
         field.setAccessible(true);
         field.set(this, input);
       } catch(IllegalArgumentException e) {  
         throw new IllegalArgumentException("caught : " + e);  
     }
   }
  }
}