Single Responsibility Principle – Does Adding a Return Type to an Update Method Violate It?

single-responsibility

I have a method that updates employees data in the database. The Employee class is immutable, so "updating" the object means actually to instantiate a new object.

I want the Update method to return a new instance of Employee with the updated data, but since now I can say the responsibility of the method is to update the employee data, and fetch from the database a new Employee object, does it violate the Single Responsibility Principle?

The DB record itself is updated. Then, a new object is instantiated to represent this record.

Best Answer

As with any rule, I think the important thing here is to consider the purpose of the rule, the spirit, and not get mired in analyzing exactly how the rule was worded in some textbook and how to apply that to this case. We don't need to approach this like lawyers. The purpose of the rules is to help us write better programs. It's not like the purpose of writing programs is to uphold the rules.

The purpose of the single-responsibility rule is to make programs easier to understand and maintain by making each function do one self-contained, coherent thing.

For example, I once wrote a function that I called something like "checkOrderStatus", that determined if an order was pending, shipped, back-ordered, whatever, and returned a code indicating which. Then another programmer came along and modified this function to also update the quantity on hand when the order was shipped. This severely violated the single responsibility principle. Another programmer reading that code later would see the function name, see how the return value was used, and might well never suspect that it did a database update. Someone who needed to get the order status without updating the quantity on hand would be in an awkward position: should he write a new function that duplicates the order status part? Add a flag to tell it whether to do the db update? Etc. (Of course the right answer would be to break the function in two, but that might not be practical for many reasons.)

On the other hand, I wouldn't nitpick what constitutes "two things". I just recently wrote a function that sends customer information from our system to our client's system. That function does some reformatting of the data to meet their requirements. For example, we have some fields that may be null on our database, but they don't allow nulls so we have to fill in some dummy text, "not specified" or I forget the exact words. Arguably this function is doing two things: reformat the data AND send it. But I very deliberately put this in a single function rather than having "reformat" and "send" because I don't want to ever, ever send without reformatting. I don't want someone to write a new call and not realize he has to call reformat and then send.

In your case, update the database and return an image of the record written seem like two things that might well go together logically and inevitably. I don't know the details of your application so I can't say definitively if this is a good idea or not, but it sounds plausible.

If you are creating an object in memory that holds all the data for the record, doing the database calls to write this, and then returning the object, this makes a lot of sense. You have the object in your hands. Why not just hand it back? If you didn't return the object, how would the caller get it? Would he have to read the database to get the object you just wrote? That seems rather inefficient. How would he find the record? Do you know the primary key? If someone declares that it's "legal" for the write function to return the primary key so that you can re-read the record, why not just return the whole record so you don't have to? What's the difference?

On the other hand, if creating the object is a bunch of work quite distinct from writing the database record, and a caller might well want to do the write but not create the object, then this could be wasteful. If a caller might want the object but not do the write, then you'd have to provide another way to get the object, which could mean writing redundant code.

But I think scenario 1 is more likely, so I'd say, probably no problem.