This Q&A is part of a weekly series of posts highlighting common questions encountered by technophiles and answered by users at Stack Exchange, a free, community-powered network of 100+ Q&A sites.

Anders Holmström asks:

Let's say I have a function IsAdmin that checks whether a user is an admin. Let's also say that the admin checking is done by matching the user id, name, and password against some sort of rule (not important).

In my head there are then two possible function signatures for this:

public bool IsAdmin(User user);

or

public bool IsAdmin(int id, string name, string password);

I most often go for the second type of signature, thinking that:

The function signature gives the reader a lot more info.

The logic contained inside the function doesn't have to know about the User class.

class. It usually results in slightly less code inside the function.

However I sometimes question this approach, and I also realize that at some point it would become unwieldy. If, for example, a function would map between ten different object fields into a resulting bool I would obviously send in the entire object. But apart from a stark example like that I can't see a reason to pass in the actual object.

I would appreciate any arguments for either style, as well as any general observations you might offer.

I program in both object oriented and functional styles, so the question should be seen as regarding any and all idioms.

See the full, original question here.

Prepare for changes

Rachel answers (102 votes):

I personally prefer the first method of just IsAdmin(User user)

It's much easier to use, and if your criteria for IsAdmin changes at a later date (perhaps based on roles, or isActive), you don't need to rewrite your method signature everywhere.

It's also probably more secure as you aren't advertising what properties determine if a user is an Admin or not, or passing around the password property everywhere. And syrion makes a good point: what happens when your id doesn't match the name / password ?

The length of code inside a function shouldn't really matter providing the method does its job, and I'd much rather have shorter and simpler application code than helper method code.

Related: "What is meant by, 'A user shouldn't decide whether it is an Admin or not. The Privileges or Security system should.'"

Don’t strew your logic about

syrion answers (67 votes):

The first signature is superior because it allows the encapsulation of user-related logic in the User object itself. It is not beneficial to have logic for the construction of the "id, name, password" tuple strewn about your code base. Furthermore, it complicates the isAdmin function: what happens if someone passes in an id that does not match the name , for example? What if you want to check if a given User is an administrator from a context where you should not know their password?

Furthermore, as a note on your third point in favor of the style with extra arguments; it may result in "less code inside the function," but where does that logic go? It can't just vanish! Instead, it's spread around every single place where the function is called. To save five lines of code in a single place, you've paid five lines per use of the function!

Transposition

GlenPeterson answers (46 votes):

Use the first, but not (only) for the reasons others have given.

public bool IsAdmin(User user);

This is type safe. Especially since User is a type you defined yourself, there is little or no opportunity to transpose or switch up arguments. It clearly operates on Users and is not some generic function accepting any int and two strings. This is a big part of the point of using a type-safe language like Java or C# which your code looks like. If you are asking about a specific language, you might want to add a tag for that language to your question.

public bool IsAdmin(int id, string name, string password);

Here, any int and two strings will do. What prevents you from transposing the name and password? The second is more work to use and allows more opportunity for error.

Presumably, both functions require that user.getId(), user.getName(), and user.getPassword() be called, either inside the function (in the first case) or before calling the function (in the second), so the amount of coupling is the same either way. Actually, since this function is only valid on Users, in Java I would eliminate all arguments and make this an instance method on User objects:

user.isAdmin();

Since this function is already tightly coupled to Users, it makes sense to make it a part of what a user is.

P.S. I'm sure this is just an example, but it looks like you are storing a password. You should instead only store a cryptographically secure password hash. During login, the supplied password should be hashed the same way and compared against the stored hash. Sending passwords around in plain-text should be avoided. If you violate this rule and isAdmin(int Str Str) were to log the user name, then if you transposed the name and password in your code, the password could be logged instead which creates a security issue (don't write passwords to logs) and is another argument in favor of passing an object instead of its component parts (or using a class method).

Find more answers or leave your own answer at the original post. See more Q&A like this at Programmers, a question and answer site for professional programmers interested in conceptual questions about software development. If you've got your own programming problem that requires a solution, log in to Programmers and **ask** a question (it's free).

