Reputation: 5528

What would you name this CRUD class?

Trying to avoid the SomethingManager trap here...

Let's say I'm going to write a User editor which will allow administrators to create users in the system. Pretty basic functionality - view a list of existing users, create a new user, update an existing user, delete a user.

Let us also say that I decide to write a "business" class to handle these basic CRUD operations. This is probably what the interface would look like:

public interface ISomeUsefulName
{
    IList<User> FetchUsers();
    User FetchUser(int userId);
    bool SaveUser(User user);
    bool DeleteUser(int userId);
}

Inside the SaveUser() method, for example, I would validate the data (using a different class) and then actually save the data to the database (again using another class).

My question is, what should I name this class? Is this class doing too much and therefore I should split it into multiple classes?

Upvotes: 10

Answers (10)

Paul Sonier

Reputation: 39510

I'd go with UserActions. This describes the set of functionality you want to do; it avoids the trap of calling it a collection (since it doesn't actually collect anything, simply retrieves a collection).

But I'd also rethink having this class in this form in the first place. It looks like what you're trying to put in place is a persistence manager; are there any other types of objects that you're going to want to persist in this manner? Can you extract any common functionality that can then be derived to a base class? Perhaps a "PersistenceManager" class or somesuch? Then, if it's absolutely necessary (and I'm not certain it would be), you could derive a "UserPersistenceManager" that would operate on User objects alone. (I believe it might not be necessary because you may be able to perform everything you need just from the PersistenceManager; only your specific implementation can tell you that, though.)

Upvotes: 0

ruslander

Reputation: 3875

Naming is difficult if is not respected SRP :) But members naming is often misused.

In your case I'll do something like this:

the responsibility of implementation is to cover specified contract of persistence
"who" is under fire

Thinks without voice - persistence is done for user and a relevant name can be IUserRepository - methods are not more than for CRUD - because of the fact that the IUserRepository is for user, is not necessary to have UserSave, UserUpdate because it brakes the generic usage manner

The Magic Is Here ... just do this:

public interface IRepository<TYPE, KEY>{
  IList<TYPE> GetAll(KEY key);
  TYPE GetById(KEY key);
  void Save(TYPE obj);
  void Update(TYPE obj);
  void Delete(Key key);
}

Is it difficult ? What to do with a custom one?

public interface IUserRepository : IRepository<User, int>
{
   IList<User> GetAllMyFavorites(ICriteria crit);
   IList<Events> GetHistoryByUser(User user);   
}

In the code using an IoC container you can do easily

public UserController {
  private _userRepository = null;
  private _eventsRepository = null;

  public UserController(IUserRepository userRepository, 
  IRepository<Events,int> eventsRepository) 
  // if you are doing here just CRUD use the generic signature
  {
    _userRepository = userRepository;
    _eventsRepository = eventsRepository;
  }

  public MarkItAsGoldPartener(int userId){
     var user = userRepository.GetById(userId);
     user.PartnerType = PartnerTypes.Gold;
     userRepository.Save(user); // the user in member name is useless
     eventsRepository.Save(new Event(){Message = "The user" + UserId + "is golden" });
  }
}

good luck :)

Upvotes: 11

Daniel Brückner

Reputation: 59705

It could become a generic interface.

ICrud<T> { }

Or inspired by IUserStore.

IStore<T> { }

Upvotes: 2

Robert Kozak

Reputation: 2033

The fact that you are having trouble naming this should be a giant red flag that it is wrong.

Single Responsibility Principle (and Interface Segregation Principle) applies here. Break it up into the various operations you need.

public interface IUserList
{
    IList<User> FetchUsers();
}

public interface IUser
{
   User FetchUser(int userId);
}

public interface IUserStore
{
    bool SaveUser(User user);
    bool DeleteUser(int userId);
}

and then it gets much simpler to name them because now only one name really applies. Trust me, if you are a designer your devs will love you for making things simple to understand and use.

Upvotes: 2

Jamie Ide

Reputation: 49301

IUserRepository or IUserServices.

Upvotes: 3

T.E.D.

Reputation: 44814

I'm seconding ChrisW's call to just name it "User".

Any time you find yourself putting the same string in the name of nearly every method, it should be removed from the method names and put in the class name.

Upvotes: 5