6
\$\begingroup\$

I am parsing a response from server, and in case it contains the fields "chunk_number"(J_ID_CHUNK_NUMBER) and "total_chunk_number"(J_ID_CHUNK_TOTAL), I want to check whether I should request another chunk or not. Not a complicated task. Yet I doubt what would be a better way to implement?

Option 1 - using try catch:

private int getNextChunkNumber(JSONObject jUpdate) {
    try {
        int current;
        int total;
        current = jUpdate.getInt(J_ID_CHUNK_NUMBER);
        total = jUpdate.getInt(J_ID_CHUNK_TOTAL);
        if (current < total) {
            return current + 1;
        }
    }
    catch(JSONException e) {
        // This is an empty exception because it merges with the default result (-1)
    }
    return -1;
}

Option 2 - using the has method:

private int getNextChunkNumber(JSONObject jUpdate) {
    int current;
    if (jUpdate.has(J_ID_CHUNK_NUMBER) &&
        jUpdate.has(J_ID_CHUNK_TOTAL)) {
        current = jUpdate.getInt(J_ID_CHUNK_NUMBER);
        if (current < jUpdate.getInt(J_ID_CHUNK_TOTAL)) {
            return current + 1;
        } 
    }
    return -1;
}
\$\endgroup\$
8
\$\begingroup\$

I would recommend that you use option 2, especially since you have the ability to avoid the exception (by using the has method).

Exceptions should be used in exceptional circumstances (e.g. You might expect getInt to throw an exception if J_ID_CHUNK_TOTAL exists but does not contain a character which can be parsed to an integer). In which case you would want to wrap the call to that in a catch.

I would also suggest that you do a null check against the jUpdate object before using it.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks. My idea is to catch an exception if the API is violated in the response (jUpdate), so if J_ID_CHUNK_NUMBER or J_ID_CHUNK_TOTAL don't exist or if they are not int, it's the same for me. Regarding the check for null - it is done in the caller method. Do you think an exceptional circumstance is only String instead of int or also the absence of the field? \$\endgroup\$ – MByD Mar 10 '12 at 9:36
  • \$\begingroup\$ That depends on your business rules, can the code which calls this continue or not if the data is invalid. That will determine whether the routine should exit with a -1 or should it throw an exception. \$\endgroup\$ – Trevor Pilley Mar 10 '12 at 10:27
5
\$\begingroup\$

I strongly recommend option 2. Like Trevor said, exceptions should only be used to handle exceptional or more or less unexpected conditions. That is not the case.

Then, in version 1, you need to explain why that JSONException was thrown/catched and that it is perfectly OK to not handle it.

My suggestion is a modified version of Option 2:

private int getNextChunkNumber(JSONObject jUpdate) {
    int current;
    if (hasNextChunk(jUpdate)) {  // <- new method for if condition
        current = jUpdate.getInt(J_ID_CHUNK_NUMBER);
        if (current < jUpdate.getInt(J_ID_CHUNK_TOTAL)) {
            return current + 1;
        } 
    }
    return NO_NEXT_CHUNK;   // a static int with value -1
}

private boolean hasNextChunk(JSONObject jUpdate) {
   return jUpdate.has(J_ID_CHUNK_NUMBER) &&
        jUpdate.has(J_ID_CHUNK_TOTAL); 
}
\$\endgroup\$

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.