Disclaimers:
For my reply, I'm going to focus on the style issue and forget about wether an ID, login and plain password is a good set of parameters to use, from the security point of view. You should be able to extrapolate my answer to whatever basic set of data you're using to figure out about the user's privileges...
I also consider user.isAdmin()
to be equivalent to IsAdmin(User user)
. That's a choice for you to make.
Reply:
My recommendation is to either user only:
public bool IsAdmin(User user);
Or use both, along the lines of:
public bool IsAdmin(User user); // calls the private method below
private bool IsAdmin(int id, string name, string password);
Reasons for the public method:
When writing public methods it's usually a good idea to think about how they will be used.
For this particular case, the reason why you'd usually call this method is to figure out if a certain user is an administrator. That's a method you need. You really don't want each caller to have to pick the right parameters to send over (and risk mistakes due to code duplication).
Reasons for the private method:
The private method receiving only the minimum set of required parameters can be a good thing sometimes. Sometimes not so much.
One of the good things about splitting these methods is that you can call the private version from several public methods in the same class. For example if you wanted (for whatever reason) to have slightly different logic for Localuser
and RemoteUser
(maybe there's a setting to turn on/off remote admins?):
public bool IsAdmin(Localuser user); // Just call private method
public bool IsAdmin(RemoteUser user); // Check if setting is on, then call private method
private bool IsAdmin(int id, string name, string password);
Additionally, if for any reason you need to make the private method public... you can. It's as easy as just changing private
to public
. It may not seem like that big of an advantage for this case, but sometimes it really is.
Also, when unit-testing you'll be able to make much more atomic tests. It's usually very nice not to have to create a full user object to run tests on this method. And if you want full coverage, you can test both calls.