Let’s say I have a model of some data:
public class User {
public String name;
public String password;
}
And some other class to work with this User:
public class ClassB {
public static void saveUser(User user) {
// Something
}
public static void saveUser(String username, String password) {
// Something
}
}
What is the better way to go? Would you better choose second method so not to couple these two classes or choose the first way just passing a User instance?
4
Consider the following (some of it is from the excellent Clean Code by Uncle Bob):
If there are direct connection between the parameters, you should encapsulate it to a class. (something like that from the book)
Example: if you have a UserCredential class, you can easily add SSO or one-time-ticket based authentication. If you had a method or property called AreValid, you won’t ever need to modify your code, even if you are adding those two new functionalities.
See?
public static void saveUser(UserCredentials credentials)
{
if (credentials.AreValid)
{
// Something - won't ever change
}
}
Note: It is not even a User class, just the credentials.
The additional benefit of creating smaller classes (UserCredentials instead of User) is: you can add logic into that small class – so finding out if the user has valid credentials won’t pollute your code where you are focusing on saving something: you are separating the responsibilities (check: Single Responsibility Principle).
2
As you’ve identified, saveUser(User user) couples the two classes. This is not always bad. If the sole purpose of class B is to save a User then it’s fair enough for it to expect a User object to save. However, it does potentially lead to extensibility issues, if the model is complex enough.
It also gives the saveUser method access to any logic you add to your User class. A low risk, but a risk nontheless.
On the other hand, saveUser(String username, String password) carries its own problems. it is very easy, especially when you have 10 or more parameters, to get them mixed up. If someone accidentally types saveUser(user.Password, user.Username) then you’re not going to get a compile-time error, or even a runtime error, it’s going to happily save those strings in the wrong fields.
Also, a very long list of parameters can make your code unreadable.
A better option is probably to use an interface saveUser(IUserDetails user) which doesn’t couple the classes to each other, but rather couples the saveUser method to any class that contains the data it requires to perform the task it has been given to do — save a username and password.
1
I would go for an object. First of all, its object-oriented but also because of readability:
User user = new User("aString", "anotherString")
saveUser(user);
is better readable than:
saveUser("aString", "anotherString");
and it’s more safe, e.g.:
saveUser(password, username) //oops
It is rarely a good idea to use multiple parameters to represent a single “object” (here used as a logically cohesive set of variables) in a function, when you can it is advisable to only pass a single parameter for each such “object”.
Consider that when you change the permission framework to use a authentication token then you’d need to change all calls that use username/password plus the implementation of those functions, compared to only passing user where you just need to change the implementation.
Or take a third option and create a sub object like an authentication token (to continue the analogy) which you can pass around, this is higher level than username/password strings but still decouples user from ClassB.