What's "best practice" here?
The best practice is to take a holistic approach with particular emphasis upon the needs of the caller. That does not necessarily mean providing maximum flexibility for the caller! It means understanding the use cases for the caller.
The way I would personally architect this solution is:
// Notice: User, not Users. This notifies a single user.
void NotifyUser(int id)
{
// notify the user
}
void NotifyUsers(IEnumerable<int> ids)
{
foreach(var id in ids)
NotifyUser(id);
}
This emphasizes to the caller that if you want to do it once, you use NotifyUser
, and if you have any sequence of users -- not just a list -- then you call NotifyUsers
with that sequence.
Now the next question I would ask is: do the callers ever want to do this:
NotifyUsers(10, 20, 30);
In that case, I would add a third function:
void NotifyUsers(params int[] ids)
{
NotifyUsers((IEnumerable<int>) ids);
}
This technique is flexible for the caller while ensuring that the majority of your methods are trivial one-liners. If there is a bug in NotifyUser you want to fix that in only one place.
A downside of that approach is that NotifyUsers()
becomes legal and is a no-op. Someone could call it accidentally and think it was doing something. In that case you might force there to be at least one:
void NotifyUsers(int id, params int[] ids)
{
NotifyUser(id);
NotifyUsers((IEnumerable<int>) ids);
}
This illustrates an important point: thinking about the needs of the caller also involves psychoanalyzing people you don't know to figure out what they are going to do wrong, and then preventing it before it happens. Designing APIs that lead people naturally to only success is hard!
(Also, note that these sketches omit error handling; you probably want to check that the sequences and arrays are not null, and so on.)
The key here is: start by understanding the caller's needs; design the API that meets their needs. Then implement it. That said, what should the implementation choice be?
It depends on how notifying a sequence of users works. The solution I sketched above makes some assumptions. They are:
- It is not more efficient to notify a hundred users "at once" than it is to notify a hundred users one at a time.
- If notification of one user in a sequence fails, the right thing to do is to stop notifying any more.
What if those assumptions are wrong? Consider an API which updates a database, and the expensive part is not the update but making the connection to the database. In that case you do NOT want to write:
void NotifyUser(int id)
{
Connect(); // Expensive
Update(id);
Disconnect();
}
void NotifyUsers(IEnumerable<int> ids)
{
foreach(var id in ids)
NotifyUser(id);
}
Because now you are doing 100 connections for 100 users. Instead you want to flip the script:
void NotifyUser(int id)
{
NotifyUsers(Enumerable.Repeat(id, 1));
}
void NotifyUsers(IEnumerable<int> ids)
{
Connect();
foreach(var id in ids)
Update(id);
Disconnect();
}
Notice that in rewriting the implementation, I did not rewrite the API; remember, we've already designed the API to be good for callers, so if this API meets their needs, don't change it! Methods are abstractions; we can change the details to meet performance requirements. (And if we cannot meet perf goals because of an API design issue, then we didn't design an API that met caller needs in the first place.)
And now what about error handling? There are numerous possibilities:
- Notifications are required to be best-effort. If one notification in a sequence fails, catch the exception, eat it, and keep going with the other users. Never inform the caller of failure
- Notifications are required to be best-effort, but callers are required to know about all failures. Log the exceptions and either re-throw them or return a failure report instead of void.
- Notifications are required to be all-or-nothing. That is, if the tenth notification fails, then undo the previous notifications to keep the world consistent. This is hard. You can't un-ring a bell.
And so on. Again think very carefully about the needs of the caller. What do they expect will happen on failure? Do you know how to write the logic to do what the caller expects? Can you clearly document it so that the caller knows whether their expectations are met?
params
keyword?userIds
or something with the collection as a whole?IEnumerable
would work.