CRUD operations; do you notify whether the insert,update etc. went well ?

Question

Hi guys.

I have a simple question for you (i hope) :)

I have pretty much always used void as a "return" type when doing CRUD operations on data.

Eg. Consider this code:

public void Insert(IAuctionItem item) {
    if (item == null) {
        AuctionLogger.LogException(new ArgumentNullException("item is null"));
    }

    _dataStore.DataContext.AuctionItems.InsertOnSubmit((AuctionItem)item);
    _dataStore.DataContext.SubmitChanges();
}

and then considen this code:

public bool Insert(IAuctionItem item) {
    if (item == null) {
        AuctionLogger.LogException(new ArgumentNullException("item is null"));
    }

    _dataStore.DataContext.AuctionItems.InsertOnSubmit((AuctionItem)item);
    _dataStore.DataContext.SubmitChanges();

    return true;
}

It actually just comes down to whether you should notify that something was inserted (and went well) or not ?

Answer 1

I typically go with the first option there.

Given your code, if something goes wrong with the insert there will be an Exception thrown.

Since you have no try/catch block around the Data Access code, the calling code will have to handle that Exception...thus it will know both if and why it failed. If you just returned true/false, the calling code will have no idea why there was a failure (it may or may not care).

Answer 2

I think it depends. Imaging that your user want to add a new post onto a forum. And the adding fail by some reason, then if you don't tell the user, they will never know that something wrong. The best way is to throw another exception with a nice message for them

And if it does not relate to the user, and you already logged it out to database log, you shouldn't care about return or not any more

Answer 3

I think it is a good idea to notify the user if the operation went well or not. Regardless how much you test your code and try to think out of the box, it is most likely that during its existence the software will encounter a problem you did not cater for, thus making it behave incorrectly. The use of notifications, to my opinion, allow the user to take action, a sort of Plan B if you like when the program fails. This action can either be a simple work around or else, inform people from the IT department so that they can fix it. I'd rather click that extra "OK" button than learn that something went wrong when it is too late.

Answer 4

I think it would make more sense if in the case where "item == null" that you returned "false". That would indicate that it was a case that you expect to happen not infrequently, and that therefore you don't want it to raise an exception but the calling code could handle the "false" return value.

As it standards, you'll return "true" or there'll be an exception - that doesn't really help you much.

Answer 5

You should stick with void, if you need more data - use variables for it, as either you'll need specific data (And it can be more than one number/string) and an excpetion mechanism is a good solution for handling errors.

so.. if you want to know how many rows affected, if a sp returned something ect... - a return type will limit you..

Answer 6

Don't fight the framework you happen to be in. If you are writing C code, where return values are the most common mechanism for communicating errors (for lack of a better built in construct), then use that.

.NET base class libraries use Exceptions to communicate errors and their absence means everything is okay. Because almost all code uses the BCL, much of it will be written to expect exceptions, except when it gets to a library written as if C# was C with no support for Exceptions, each invocation will need to be wrapped in a if(!myObject.DoSomething){ System.Writeline("Damn");} block.

For the next developer to use your code (which could be you after a few years when you've forgotten how you originally did it), it will be a pain to start writing all the calling code to take advantage of having error conditions passed as return values, as changes to values in an output parameter, as custom events, as callbacks, as messages to queue or any of the other imaginable ways to communicate failure or lack thereof.