What is a better oo design scenario and why?

Question

If I have a User and I want that user to Register, is it better to have a Registration class with a Register method that accepts a user or is it just good enough to have a User object that has a Register method

public class Registration
{
    public void Register(User user)
    {

    }
}

public class User
{
    public void Register()
    {

    }
}

What are the advantages of doing it one way over the other?

Answer 1

I would prefer the first, as the second makes a tightly-coupled assumption that the User knows what to register for, and that Register() will not mean different things down the inheritance road.

Answer 2

This question is far too vague to answer. I don't know what "Register" means in this context, but is it possible that you might be able to register more than just a user?

Would it be smart to do:

public interface IRegisterable
{
    // ... something makes me registerable....
}

public class User : IRegisterable
{
}

public class Register
{
   public void Register(IRegisterable item) { }
}

I don't know because you haven't told us enough to answer the question. Are there multiple registrations available for instance? Is the user registering for something, and might register for something else later?

Answer 3

The way I'd look at it is to think in terms of real life objects. Is a 'Registration' an object, a real 'thing'? Probably not.

The real 'things' you have in this scenario are the User, and the thing they're registering with (I'll assume you're talking about a website, for example).

So - I'd probably create an object which represents the thing the user is registering with - 'Site', for example, and add a Register method on that.

Answer 4

I think better oo design would be

public class UserRegistry {
    public void Register(User user) {...}
}

or public class MyApplicationClient { public void Register(User user) {...} }

Even better might be (probably an overkill, and does not work for your case at all):

public class UserRegistrar {
    public void Register(User user, IUserRegistry userRegistry) {...}
}

public class MyApplicationClient : IUserRegistry {}

public IUserRegistry {
    // Add, Remove, IsRegistered
}

Your first option could translate in English as: "A registration that can register a user." But registration (record) is a product of registration (process).

Second option requires user to know context it to be registered in, you don't say "hey, user - go register yourself with that department." you say "hey department, here is a user - register him."

So method Register() shall act on user, user is being registered, it shall not contain details on how it is registered (simple test: registration can be different depending on the context user is to be registered in, therefore user shall be agnostic to details of registration).

Answer 5

Having the User being able to Register, means the User class must have access to some form of registration system. It is therefore tightly bound to some form of registration system. What if you wanted to re-use the User class for administrators who register with a different system?

Now I probably wouldn't use the below, but it's just a quick brain-fart on how Authentication or Authorisation is not the responsibility of a User. It's the responsibility of the system, or a sub-system within the system.

class User : IMember {} 
class Group : IMember {}

class Resource : IResource {}

// Assumes the User is already Authenticated in some way...
class Authorisation
{
    static bool IsAuthorised(IMember member, IResource resource) {}
    static bool IsDenied(IMember member, IResource resource) {}
    static bool IsAnyDenied(IEnumerable<IMember> members, IResource resource) {}
    static bool IsAnyAuthorised(IEnumerable<IMember> members, IResource resource) {}
}

class System
{
    bool CanEnterAdminArea(User user, IResource admin) 
    {
        IEnumerable<Group> groups = u.Groups;

        if ( Authorisation.IsAnyDenied( groups, admin ) { return false; }
        if ( Authorisation.IsDenied( user, admin ) { return false; }

        return (Authorisation.IsAuthorised( user, admin ) 
             || Authorisation.IsAnyAuthorised( groups, admin ));


    } 

}