BraveHeart BraveHeart - 3 months ago 7
C# Question

Does having a method that updates and add new record if the record does not exist break SRP?

Lets say I have a DatabaseService class that has Update(user) and Add(user).
My question is about having them both in one method. Is it a good practice or not ?
To be more specific does it break the singularity rule (i.e. Single Responsibility)?
For example if i have a mothed like this

public void AddOrUpdateUser(User user)
{
var userFound = GetUserById(user.Id);
if (userFound == null)
{
AddUser(user);
}
else
{
UpdateUser(user);
}
}


Would it be a good idea ? or would it be bad because it breaks the singularity rule so let the consumer class decides what to do?

Answer

It's common to work this way. If given user it hasn't been already persisted, it'll be created, otherwise, it'll be updated.

BTW, I would change the part of getting a possible user by given Id with something like this:

public interface IPersistable
{
     bool IsNew { get; }
}

public class User : IPersistable
{
    public int Id { get; set; };
    public bool IsNew => Id == 0; 
}

And then:

IPersistable persistable = user as IPersistable;
Contract.Assert(persistable != null);

if(persistable.IsNew)
{
    // Create
}
else
{
   // Update
}

OP concerns about single responsibility principle (SRP)

Since we call this method as AddOrUpdate, OP has said in some comment on my answer that this could break single responibility principle.

I really believe that this concern comes from the method identifier rather than of what the method really does.

The whole method takes care of persisting objects and, from the consumer point of view, this is a single responsibility: I want to you to take care about persisting my object. Either if it was already on the underlying store or not, it's still persisting an object. We could rename this method to Persist, Save or whatever, but AddOrUpdate, at least, provides more semantics to the operation and it follows another unofficial (but widely accepted) principle: self-documented code (you can understand what your code does just by reading the code itself).

Thus, it seems like it would break single responsibility principle if it would also delete objects or any other unrelated operation that could be represented by another verb.