Logical mistake or not?

I have written this function which will set val=max or min (if val comes null) or val=val (val comes as an Integer or "max" or "min")

while calling i am probably sending checkValue(val,"min") or checkValue(val,"max")

 public String checkValue(String val,String valType)
 {

   System.out.println("outside if val="+val);

   if(!val.equals("min") && !val.equals("max"))
     {
     System.out.println("Inside if val="+val);

     try{
         System.out.println("*Inside try val="+val);
         Integer.parseInt(val);
         }

     catch(NumberFormatException nFE)
         {
         System.out.println("***In catch val="+val);
         val=valType;
         }

     return val;
     }
     else
     {
       return val;
     }
}

But the problem is if val comes null then outside if******val=null is shown. Ca开发者_如何转开发n any1 tell me is this a logical mistake? And why will I correct?

---

If val is null, then the expression val.equals("min") will throw an exception.

You could correct this by using:

if (!"min".equals(val) && !"max".equals(val))

to let it go inside the if block... but I would personally handle it at the start of the method:

if (val == null) {
  // Do whatever you want
}

Btw, for the sake of readability you might want to consider allowing a little more whitespace in your code... at the moment it's very dense, which makes it harder to read.

---

...the problem is if val comes null then outside if****val=null is shown. Can any1 tell me is this a logical mistake?

The output is correct; whether you want it to come out that way is up to you.

Your next line

if(!val.equals("min") && !val.equals("max")){

...will throw a NullPointerException because you're trying to dereference val, which is null. You'll want to add an explicit check for whether val is null:

if (val == null) {
    // Do what you want to do when val == null
}

---

you should use valType instead of val to check either minimum or maximum is necessary to check.

My advice to you in such cases to use boolean value or enum instead of strings. Consider something like that:

/**
 * check the value for minimum if min is true and for maximum otherwise
 */
public String checkValue(String val, boolean min){
   if (min) {
      // ...
   } else {
      // ...
   }
}

---

If you need to compare strings against constants you should write it the other way around to make it null-safe:

 if (! "min".equals(val))

And while this is mostly a style issue, I would make all method arguments final and not re-assign them (because that is confusing), and you can also return from within the method, not just at the end. Or if you want to return at the end, do it at the very end, not have the same return statement in both the if and the else branch.