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

Is it bad when my code is throwing exceptions, but doesn't crash during runtime?

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

Problem

Here's part of code in Java for battleships game I'm working on:

lblGrid2 = new JLabel(new ImageIcon(myPicture));
    lblGrid2.addMouseListener(new MouseAdapter() {
        @Override
        public void mouseClicked(MouseEvent e) {
            for (int i = 0; i < shipList.size(); i++)
                if (shipList.get(i).isActive())
                    shipList.get(i).place();
        }
    });
    lblGrid2.addMouseMotionListener(new MouseMotionAdapter() {

        @Override
        public void mouseMoved(MouseEvent m) {

            for (int i = 0; i < shipList.size(); i++) {
                if (shipList.get(i).isActive()) {
                    shipList.get(i).moveShip(m.getX(), m.getY());
                }
            }
        }
    });
    lblGrid2.setText("");
    lblGrid2.setBounds(40, 40, 401, 401);
    panel.add(lblGrid2);


When placing ships on board. this listens for mouse movement and should move ship image accordingly, but that happens only after the client and server are connected and there is an active ship that can be placed. If that's not the case, for example I haven't connected to the server yet, every mouse move throws an exception.

Is this something I should be concerned about?

Solution

Yes, absolutely, you should be concerned.

Exceptions should be exactly that, exceptional. and should never be part of the regular code path. If there are circumstances that you know would be normal (like not yet being connected to a server) then you should code in a condition that represents that situation, and make your code a conditional-execution path. For example, in your code, I would have an AtomicBoolean that represents the 'connected' state, and then only process mouse-events when connected.

As a side-note, Exceptions in Java are very 'expensive'. Almost all Java VM's (OpenJDK, Oracle JDK, IBM JDK, Android JDK, etc.) need to inspect the entire call-graph of the code when an exception happens, so that it can build up the stack trace for the exception. In all the VM's I know, that requires that:

  • The entire VM thread is 'suspended' so that the call-graph does not change



  • all threads are 'paused'



  • the call stack is inspected and walked up.



  • all references on the call-stack are chased back in to the source (get line numbers, etc.)



This is very expensive to process. Using exceptions as part of the normal program flow is a bug, and preventing exceptions with conditional statements is always a good idea. For example, the following code example is terrible:

int val = 0;
boolean isNumber = false;
try {
   val = Integer.parseInt(somestring);
   isNumber = true;
} catch (NumberFormatException nfe) {
   // ignore not-numbers, and leave isNumber to be false
}


I see code like the above all over the place.... and it is broken. In one example, I have seen a 50% performance improvement on a CSV reading program by changing code like that to be:

int val = 0;
boolean isNumber = false;
if (someString.matches("\\d+")) {
    try {
       val = Integer.parseInt(somestring);
       isNumber = true;
    } catch (NumberFormatException nfe) {
       // ignore not-numbers, and leave isNumber to be false
    }
}


Even though the code does extra work to pre-validate that the string contains digits, it is faster! (and is even faster if you precompile the Match pattern...!)

So, in your code, I would have:

private final AtomicBoolean isConnected = new AtomicBoolean(false);


and then, in your code:

lblGrid2.addMouseMotionListener(new MouseMotionAdapter() {

    @Override
    public void mouseMoved(MouseEvent m) {

        if (!isConnected.get()) {
            return;
        }

        for (int i = 0; i < shipList.size(); i++) {
            if (shipList.get(i).isActive()) {
                shipList.get(i).moveShip(m.getX(), m.getY());
            }
        }
    }
});


Some discussions about Java Stack traces and exceptions:

  • Longjumps Considered Inexpensive



  • How slow are Java exceptions?

Code Snippets

int val = 0;
boolean isNumber = false;
try {
   val = Integer.parseInt(somestring);
   isNumber = true;
} catch (NumberFormatException nfe) {
   // ignore not-numbers, and leave isNumber to be false
}
int val = 0;
boolean isNumber = false;
if (someString.matches("\\d+")) {
    try {
       val = Integer.parseInt(somestring);
       isNumber = true;
    } catch (NumberFormatException nfe) {
       // ignore not-numbers, and leave isNumber to be false
    }
}
private final AtomicBoolean isConnected = new AtomicBoolean(false);
lblGrid2.addMouseMotionListener(new MouseMotionAdapter() {

    @Override
    public void mouseMoved(MouseEvent m) {

        if (!isConnected.get()) {
            return;
        }

        for (int i = 0; i < shipList.size(); i++) {
            if (shipList.get(i).isActive()) {
                shipList.get(i).moveShip(m.getX(), m.getY());
            }
        }
    }
});

Context

StackExchange Code Review Q#51616, answer score: 12

Revisions (0)

No revisions yet.