Is it better for an API when it provides additional methods that check if something exists before adding?

For example:

if (!userManager.HasPermission(user, "somePermission")
   userManager.AddPermission(user, "somePermission");

versus

userManager.AddPermisionIfDoesntExist(user, "somePermission");

If you use this HasPermission in many places, wouldn't it be better to create method like AddPermisionIfDoesntExist ?

Or, if duplicates aren't allowed, should AddPermission check it instead?

So that would be similar to how Dictionary is designed, with methods like TryAddPermission and AddPermission that throws exception when permission already exist?

also, note that AddPermission and HasPermission are doing calls to the database

  • It really depends on what AddPermission should do when it already exists? Should it overwrite existing? Should it fail? Should I do nothing? – Euphoric Dec 6 at 14:29
  • @Euphoric there is nothing to overwrite, because it should act more like Dictionary, so preferably it should do nothing or at least indicate that something already exists similarly to how Dictionary does it. – Konrad Dec 6 at 14:31
  • You might be interested in the Tell, Don't Ask principle. Ultimately, it depends on the kind of API you want, but as a rule of thumb, I would follow this principle. Note that your first version may have concurrency issues, so it might be worth taking this into account. – Vincent Savard Dec 6 at 16:53

The correct answer here depends greatly on whether the resource being modified is shared.

if (Files.exists(path)) {
    Files.delete(path);
} catch (NoSuchFileException x) {
    System.out.println("Er wait, how'd we get here?");
}

Here we're confronted with the fact that we're not the only process with access to this resource. If we had it to ourselves we could avoid the nasty NoSuchFileException exception.

So what to do? Well we can lock the resource to get an atomic "test and set" combo going so we can control what's happening or we can just make the API do all that.

Files.deleteIfExists(path);

Here knowledge of whether the resource existed is being returned, and ignored since we don't happen to care.

However, if we know ahead of time that we don't care then this

userManager.AddPermision(user, "somePermission");

has no reason to throw when the permission already exists. It just has a bad name. For that case I'd prefer to see this:

userManager.SetPermision(user, "somePermission");

After that I'd feel fine assuming that whatever else is going on that permission is now set.

Its usually best if the API is written so that the check itself is internal, but it depends on what the behaviour you want from both the API and the caller.

  • Hide it all behind the API where caller shouldn't really care and its more of an internal implementation thing
    Permission has no other properties: addPermission(user, 'somePermission')
    And we are ok if this call just creates the permission entry (some sort of flexible admin schema)

  • throw an exception if the caller must have done something else first (eg, set permissions which shouldn't be under program control)
    Permission can't be added dynamically try {addPermission(user, 'somePermission') } catch e(){alert ("permission doesn't exist")}

  • Provide an override method if you want the caller to optionally ask the API to do the job on their behalf (and presuming this is practical for the API to do for them)
    addPermission(user, 'somePermission', addIfMissing=true)
    Again, we have to be ok if this call just creates the permission entry

  • If additional properties are required for the precursor then expose extra methods for the caller to create the thing.
    If you don't expose an existence check then the caller should implement a try/catch around the main method call just in case they need to make the precursor call. Better to expose the existence check as well so they can make it obvious in their code:

if(!permissionExists("somePermission")
{createPermission("somePermisson",newPermissionName)}
addUserPermission(user,"somePermission")

Support both workflows.

Some callers will want to use HasPermission() to validate assumptions before continuing with a long or expensive transaction — or to otherwise validate assumptions or debug. This method should be more accessible than AddPermission() to support those workflows.

However, if a client chooses to call AddPermission() without checking for existence first, don't force them catch exceptions or differentiate between types of success. And to that end, return a response object that can communicate the nuanced success and failure modes. Something like:

{
  success: true|false,
  status: Added|AlreadyExists|CannotAdd|NoPermissions|etc..,
  message: "Description/Guidance regarding the status code."
}

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.