-2

I have joined writing middle-size multi-purpose database application as co-lead. It currently has about 150 tables (and growing) and overall functionality you can imagine as very small ERP.

At many places in code, functionalities which would normally belong into procedures (I mean functions without return value) like DeleteAttachment or SaveRecord are put into functions with return value reporting their exit status in string. They return either "OK" on success or messages like "Deletion failed because ......." or "Insufficient privileges for .......".

At the end, often the end user is presented with message they provided. On one hand, this is effective mechanism of tracing of potential problems. On the other hand, I'm not sure whether I should encourage this approach of writing the inner business logic.

I understand that putting non-fatal problems into exceptions is also not very good way to go (EInsufficientUserPrivileges, ERecordAlreadyExists). Should we stick with current approach or is there some other which is more suitable for applications using a many checks and business logic?

  • EInsufficientUserPrivileges, ERecordAlreadyExists look like fatal problems to me - you do not get your original action completed, do you ? As such, I would make a case for throwing exceptions. – mika Apr 11 '14 at 11:03
  • @mika - If you think most of error paths should be put into exceptions (and functions should return nothing), please create an answer based on your comment so it can be commented separately. – miroxlav Apr 11 '14 at 11:13
  • EInsufficientUserPrivileges, ERecordAlreadyExists look like some pretty common paths to me so you should write code for it to handle the condition, not throw an exception, because expected exceptions should never be exceptions but handled. – Pieter B Apr 11 '14 at 11:37
  • @PieterB - you answer is partially an opposite to what mika says (all error paths should create exceptions). Could you draw the line between exceptions and return values in your own answer? – miroxlav Apr 11 '14 at 11:51
3

To draw the line between exceptions and return values, I would try to stick to the following: - Anytime an instruction breaks the expected flow of execution, you expect an error to be thrown. - If the expected function of procedure may have different outcomes, you will want to work with return values.

In the examples that you are stating, it seems very clearly that the expected instruction is not performed. Error messages like: "Deletion failed", or "Insufficient privileges" end up with your action being void. Therefore, you would clearly expect an error to be thrown. In this context, fatal is equivalent to "your instruction could not be performed". On the other hand, an exception does not necessarily have to be fatal. You may well handle it, and proceed with other instructions (other deletions, insertions, or whatever needs to be done). This is what you can achieve with a catch block.

Another thing: returning a string with "OK" is not good practice. Use boolean values instead. Now again, if you want return values to be "OK" or "Error message", a boolean will not let you achieve that. An exception will let you put an error message, and any detail you want to store about the error that has happened. You could for example, if a user is denied access, store the user name, the priviledges that he has, and the record that he tried to access in an exception. You could then use this information as you catch the error to generate a short error message to the user, and a more detailed message for your error log.

  • I see this is going to require many different types of custom exceptions. – miroxlav Apr 11 '14 at 11:53
  • 1
    "Anytime an instruction breaks the expected flow of execution, you expect an error to be thrown" -- this is totally arbitrary and subjective. I expect that different programmers would have different expectations. – user39685 Apr 11 '14 at 11:54
  • @MattFenwick - I must agree with mika that exceptions could provide effective reporting if a problem occurs. Is there other effective mechanism to provide error reporting? Things like GetLastError() can be tricky, I wouldn't go that way. Returning simply true/false is not informative about what happened. – miroxlav Apr 11 '14 at 11:59
  • 2
    @mika The problem is that you can always shift the semantics around to say that failure is an expected outcome. You could argue that failing to open a file is not the expected outcome, and I could argue that by definition there's no guarantee a file will be there so failure to open it is expected. The real distinctions between error codes and exceptions is that error codes make failure a part of the function's type (for better and worse) while exceptions facilitate handling the error several layers up the call stack (as opposed to repeatedly returning error codes all the way up.) – Doval Apr 11 '14 at 13:12
  • 1
    I'm reading Code Complete Second Edition and chapter 8.4 clearly states that exceptions should be reserved for conditions that are truly exceptional—in other words, for conditions that cannot be addressed by other coding practices. Exceptions are used in similar circumstances to assertions—for events that are not just infrequent but for events that should never occur. Exceptions without proper handling also weaken encapsulation:low-level exception can be propagated into upper contexts. Recommendation is to use other error-handling mechanisms, too. – miroxlav Apr 11 '14 at 15: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.