0

I recently got into discussion about how the method signature should look like for an API method which doesn't expose internal objects used by application to caller. Here is how situation looks:

  1. Our API should expose one method say,

    public int updateSummary(String summaryMessage) {
    
          update_in_DB(summaryMessage);
          if (success) return 1;
          return -1;    }
    

or,

    public void updateSummary(String summaryMessage) {

          update_in_DB(summaryMessage);
         }

This method can be called by clients to update summary of operations.

  1. The debate was, what should be Return_Type?

    i. void or,

    ii. something like int or, enum which conveys information around whether that operation was successful or not.

    iii. Return_Type can't be of type object/entity which represents actual object because it will expose internals of application to caller.

my point is, adding a return type like int/enum will make method testable (via unit testing) also, it enables caller to take action if this operation fails. But counter part to this was given that, it should be void and if operation fails error should get propagated to user because application should throw error indicating that something is wrong in deployment. Is anyone aware of any best practices or standards around this?

1

Exceptions can happen even in good code (e.g.: Oh no, server X has gone down and is not responding to attempts to save to DB), so I agree exceptions should always be propagated back.

That doesn't answer the return type question though. Is there a scenario whereby the database layer code might decide to NOT save (is validation happening here?) should a meaningful return type be returned? Examples might include a lack of authority to update that object, or a bad bit of data? If it is a simple Saved OK vs Exception then void is quite acceptable as a return type. If some sort of failure reason code response is required then a non-void response would be appropriate, a response code enumeration perhaps.

It isn't clear how "pure" your design is and if Update_in_DB is accepting data already validated and checked for authorisation as a pure DB action or if further checks will happen in this call.

  • 1
    what about testability of this code? should this be based upon assumption that if no exception occurred then all is fine. What we will loose if we just return an integer saying "its a failure". – Prateek Jain Oct 24 '18 at 9:28
  • Why would it fail - if it fails because bad data has been passed in that is being validated further down the line then that is the "need a meaningful return code" scenario - in which case integer/boolean/enum? make a your-scenario-specific decision. If it fails because of a genuine database layer error you'd be throwing back up exceptions right? and the test would fail because of the exception? – SazooCat Oct 24 '18 at 10:11
  • Nope, IMO it should behave like API. Client asked for something to be saved in DB. API should return success or failure code rather than throwing exception trace, which would expose internal functionality too. – Prateek Jain Oct 24 '18 at 11:13
  • What are clients supposed to do with 1/-1? What -1 does mean? How consumers are supposed to handle a -1? I agreed with SazooCat. If you only need to communicate failure, Exception/Error is a good way to do it. They also make code testable. Checked or not checked is just another subject. If you need to provide with information regarding the whole transaction (some db drivers do it ), 1/-1 is not probably the best approach. It seems code smell to me. In other words, 1/-1 leaks on meaning, in consequence, consumers can do very little to recover from the error. – Laiv Oct 24 '18 at 12:59
0

IMHO, there are two different concepts here. One is if there’s anything meaningful you should return on your method in case the update succeeds, and that depends on the rest of the flow.

The other is how you should handle an exception thrown in the update process. There the good practice is usually catch the exception and raise another, more meaningful exception to let the client deal with.

For example, one client might have to abort if a bad update happens, other might retry, an third might ignore it...

That’s how I usually handle it, anyway, hope it’s helpful!

  • Thats the whole point, IMO it shouldn't be driven by execption handling but clean return codes like success or failed. – Prateek Jain Oct 24 '18 at 11:15
  • You seem to have already made your mind, which kind of defeats the purpose of discussing it here. What I can tell you is most language creators don’t agree with you, you might guess they know their business. Code for the win, handle the exceptions, instead of checking success for every single action you do. Happy coding! – Tomaz Fernandes Oct 24 '18 at 15:07
  • I am looking for testable approach which is also easier for clients too. – Prateek Jain Oct 24 '18 at 17:19
  • You can easily unir test exception handling too with mocks... For example, you mock the repository object and program the mock to throw an exception when accessed, or pass a wrong parameter to the method and verify that the appropriate exception was thrown – Tomaz Fernandes Oct 24 '18 at 19:30
  • I just came across junit5, assertThrows method. To me this appears to be a fair approach. – Prateek Jain Oct 27 '18 at 10:57

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.