Dtyjlui

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.

I think you might be missing the point of that try/catch. The code is using the exception system to bubble any exception message to the caller. This could be deep inside a nested call stack--not just the one "throws" you are looking at.

In other words, the "throws" declaration in your example code is taking advantage of this mechanism to deliver a message to the client, but it almost certainly isn't the primary intended user of the try/catch. (Also it's a sloppy, kinda cheap way to deliver this message--it can lead to confusion)

This return value isn't a great idea anyway because Exceptions often don't have messages and can be re-wrapped... it's better than nothing though. Exception messages just aren't the best tool for this, but handling an exception at a high level like this is still a good idea.

My point is, if you refactor this code be sure to look for runtime exceptions that might be thrown anywhere in your code base (at least anywhere called during message processing)--and even then you should probably keep the catch/return message as a catch-all just in case a runtime exception pops up that you didn't expect. You don't have to return the error "Message" as the message of your response--It could be some quippy "We couldn't process your request at this time" instead, but be sure to dump the stack trace to a log. You are currently throwing it away.

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;

}

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, I have a 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 which I mentioned before 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)

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.