0

Is it considered bad practice to do this:

Scanner scan = new Scanner(System.in);
if(scan.nextInt() == 5) {                //testing if input is equal to 5
    System.out.println("input equals 5");
}

What about:

Scanner scan = new Scanner(System.in);
if(scan.nextInt() == scan.nextInt()) {    //testing if two inputted ints are equal to each other
    System.out.println("input1 equals input 2");
}

I read somewhere that this can cause "unexpected results" but I don't know what was meant by that. I have tested this quite a bit and haven't run into anything unexpected.

6
  • 1
    Where did you read this?
    – Keppil
    Sep 28, 2014 at 18:42
  • 5
    No it isn't. Perhaps you have misunderstood what you've read.
    – Alboz
    Sep 28, 2014 at 18:43
  • @Keppil I don't remember exactly, but it was somewhere on the internet, so its reliability is questionable.
    – null
    Sep 28, 2014 at 18:43
  • 2
    This can cause unexpected results if the methods aren't "pure". For example, if you decided to add another test to check whether scanner.nextInt() == 7, the test would read another value from the Scanner. This is a very common bug in the usage of Iterator for example. So it's fine if you know want you're doing, but if you forget then... Sep 28, 2014 at 18:47
  • @Alboz Thanks, now that I think about it, I read this on a random website, and its reliability is probably not so great.
    – null
    Sep 28, 2014 at 18:47

3 Answers 3

6

The problem here is not that you call a method inside the condition of the if block. The problem is that your function is not pure, i.e., it has a side effect. If you have a look at @Krayo's example, you'll see how two seemingly equivalent pieces of code can have different behavior: && does only execute the second expression, if the first expression evaluates to true. Analogous, || only executes the second expression, if the first is false.

Have a look at the Command and Query separation principle. It states that your methods should either compute and return a value, or they should change the state of an object, but not both.

Edit: Also, have a look at the Uniform Access principle. The semantics of your program should not depend on whether you obtain a value through storage or through computation. If your program behaves differently when accessing a field than when executing a method that computes the same value, then you should probably fix your method. Note: behavior and performance are two pairs of shoes: Accessing a field will be faster than calculating a value.

5

It isn't bad practice to call a method inside an if/else. Its fine especially when it returns a value you only need that once.

1

An example of an unexpected result:

public static int count;

public static void run1() {
    count = 0;
    long r1 = getNum1();
    long r2 = getNum2(); // it is called in any case, so count = 1
    if (r1 == 1L && r2 == 0L) { /* if r1 = 1 and r2 = 0 then print "blah"
                                   if r1 = 0 then r2 is not checked! (lazy evaluation) */
        System.out.println("blah");
    }
    System.out.println(count); // always print 1 (count = 1)
}

public static void run2() {
    count = 0;
    if (getNum1() == 1L && getNum2() == 0L) { /* if getNum1() = 1 and getNum2() = 0 then print "blah"
                                                 if getNum1() = 0 then getNum2() is not checked, so count = 0! */
        System.out.println("blah");
    }
    System.out.println(count); // depends on the getNum1() result
}

// return 0 or 1 depending on milliseconds of the current time is even or odd
public static long getNum1() {
    return System.currentTimeMillis() % 2L;
}

// same as getNum1() and addition set count to 1
public static long getNum2() {
    count = 1;
    return System.currentTimeMillis() % 2L;
}
3
  • Sorry, I don't completely understand what is going on in this code (I am a beginner in Java). Could you explain what the code does and how it would cause an unexpected result?
    – null
    Sep 28, 2014 at 19:48
  • 1
    I expanded it with some comments.
    – Krayo
    Sep 28, 2014 at 20:44
  • Oh, I see the potential problem, thank you very much!
    – null
    Sep 28, 2014 at 21:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.