Sstrhsrtj

Here is the code:

public Response getABC(Request request) throws Exception {

    Response res = new Response();

    try {

        if (request.someProperty == 1) {

            // business logic

        } else {

           throw new Exception("xxxx");

        }

    } catch (Exception e) {

        res.setMessage(e.getMessage); // I think this is weird

    }

    return res;

}

This program is working fine.
I think it should be redesigned, but how?

It makes no sense to throw an exception in a try block and immediately catch it, unless the catch block throws a different exception.

Your code would make more sense this way:

public Response getABC(Request request) {

    Response res = new Response();

    if (request.someProperty == 1) {

        // business logic

    } else {

        res.setMessage("xxxx");

    }

    return res;

}

You only need the try-catch block if your business logic (executed when the condition is true) may throw exceptions.

If you don't catch the exception (which means the caller will have to handle it), you can do without the else clause:

public Response getABC(Request request) throws Exception {

    if (request.someProperty != 1) {

        throw new Exception("xxxx");

    }



    Response res = new Response();

    // business logic

    return res;

}

if you are throwing the exception from the method then why bother catching it ? it's either you return a response with "xxxx" message or throw an exception for the caller of this method to handle it.

public Response getABC(Request requst) {

    Response res = new Response();

        if(request.someProperty == 1){

            //business logic

        else{

           res.setMessage("xxxx");

        }

    }

    return res;

}

OR

public Response getABC(Request requst) throw Excetpions {

    Response res = new Response();

        if(request.someProperty == 1){

            //business logic

        else{

           throw new Exception("xxxx");

        }

    return res;

}





public void someMethod(Request request) {

    try {

        Response r = getABC(request);

    } catch (Exception e) {

        //LOG exception or return response with error message

        Response response = new Response();

        response.setMessage("xxxx");

        retunr response;

    }



}

First and foremost, tread more carefully when you refactor a working method - especially if you are performing a manual refactoring. That said, introducing a variable to hold message may be one way of changing the design:

public Response getABC(Request requst) throw Excetpions {

    String message = "";

    try{

        if(request.someProperty == 1){

            //business logic

        else{

           message = "xxxx";

        }

    }catch(Exception e){

        message = e.getMessage();

    }

    Response res = new Response();

    res.setMessage(message);

    return res;

}

The assumption is that the business logic does it's own return when it succeeds.

Why did you use try/catch statement when you already throw Checked Exception?

Checked exception is usually used in some languages like C++ or Java, but not in new language like Kotlin. I personally restrict to use it.

For example, a have class like this:

class ApiService{

    Response getSomething() throw Exception(); 

} 

which feels clean and readable, but undermines the utility of the exception handling mechanism. Practically, getSomething() doesn't offen throw checked exception but still need to behave as it does? This works when there is somebody upstream of ApiService who know how to deal with the unpredictable or unpreventable errors like this. And if you can really know how to deal with it, then go ahead and use something like the example below, otherwise, Unchecked Exception would be sufficient.

public Response getSomething(Request req) throws Exception{

    if (req.someProperty == 1) {

        Response res = new Response();

        // logic 

    } else {

        thows Exception("Some messages go here")

    }

}

I will encourage to do in this way:

public Response getSomething(Request req){

if (req.someProperty == 1) {

        Response res = new Response();

        // logic 

        return res;

    } else {

        return ErrorResponse("error message"); // or throw RuntimeException here if you want to

    }

}

For more insights, Kotlin doesn't support Checked exception because of many reasons.

The following is an example interface of the JDK implemented by StringBuilder class:

Appendable append(CharSequence csq) throws IOException;

What does this signature say? It says that every time I append a string to something (a StringBuilder, some kind of a log, a console, etc.) I have to catch those IOExceptions. Why? Because it might be performing IO (Writer also implements Appendable)… So it results into this kind of code all over the place:

try {

    log.append(message)

}

catch (IOException e) {

    // Must be safe

}

And this is no good, see Effective Java, 3rd Edition, Item 77: Don't ignore exceptions.

Take a look at these links:

• Checked and unchecked exception


• 
  Java's checked exceptions were a mistake (Rod Waldhoff)


• 
  The Trouble with Checked Exceptions (Anders Hejlsberg)

it seems doesn't make sense when purposely throwing exception and then directly catch it,
it may redesign like this,

may change "throw new Exception("xxxx");" with "res.setMessage("xxxx");"

and then may keep catching the exception part in order to catch exception that may happen inside the business logic.

public Response getABC(Request requst) {

  Response res = new Response();

  try{

      if(request.someProperty == 1){

          //business logic

      else{

         res.setMessage("xxxx");

      }

  }catch(Exception e){

      res.setMessage(e.getMessage);

  }

  return res;

}

The exception mechanism has three purposes:

1. Immediately disable normal program flow and go back up the call stack until a suitable catch-block is found.


2. Provide context in form of the exception type, message and optionally additional fields that the catch-block code can use to determine course of action.


3. A stack trace for programmers to see to do forensic analysis. (This used to be very expensive to make).

This is a lot of functionality for a mechanism to have. In order to keep programs as simple as we can - for future maintainers - we should therefore only use this mechanism if we really have to.

In your example code I would expect any throw statement to be a very serious thing indicating that something is wrong and code is expected to handle this emergency somewhere. I would need to understand what went wrong and how severe it is before going on reading the rest of the program. Here it is just a fancy return of a String, and I would scratch my head and wonder "Why was this necessary?" and that extra effort could have been better spent.

So this code is not as good as it can be, but I would only change it if you had the time to do a full test too. Changing program flow can introduce subtle errors and you need to have the changes fresh in your mind if you need to fix anything.