7

I stumble across this case somewhat often, and I'm surprised about finding so few similar discussions around the web. This question is very related, but my problem is that I want a method that does the more general "do X if Y" rather than "do X if needed". The answer in that link is to use the prefix Ensure, but that word does not fit if ensuring X is not the method's intention.

The scenario I have in mind is this:

void mayPerformAction() {
    // Do some preparatory calculations
    // ...
    if (shouldPerform) {
        // Perform action
        // ...
    }
}

The reason I am not using two separate methods (shouldPerformAction() and performAction()) is because both the condition and the action depend on some preparatory calculations, which would have to be repeated otherwise. Now, my question is: what is the most logical and readable name for the method mayPerformAction()?

To clarify, it is important to the caller that the action may sometimes not be executed, otherwise it seems logical to me to use performAction().

I admit that this is kind of an XY-problem, and there are multiple solutions posted, each of which have good arguments for and against them. To summarize:

  • Abstract away the doubt and give it a less detailed name, e.g. just performAction().
  • Prefer clarity and do the calculations twice; the performance difference will be negligible in many cases anyway: if (shouldPerform()) performAction().
  • Same as above, but store the shared result of the calculations in a global variable or return it (I prefer the latter) so no resources are wasted.

I feel like the best approach depends on how 'serious' the condition is and how expensive the preparatory calculations are; as such I'm leaving the question unanswered for now.

  • 2
    While I agree wholeheartedly with CandiedOrange's answer below, there is some precedent for, for example, a method that fills a buffer with needed data, but does nothing if the buffer is already filled. I prefix such methods with the word "Ensure," as in EnsureBufferFilled(). – Robert Harvey Feb 5 '18 at 3:28
  • 1
    @RobertHarvey I've written ensure methods myself. Heck I've written method names with if in them. But I'll take a good abstraction over either if i can find one. Sadly I can't always find one. If I can't, well, I'll take your ensure over ifXthenY() any day. :) – candied_orange Feb 5 '18 at 4:33
  • @CandiedOrange: It is, in fact, the only time I've ever put "condition words" in the name of a method that didn't return a boolean. – Robert Harvey Feb 5 '18 at 16:07
  • I've used ...IfRequired(...) in addition to Ensure...(...) when there's no good alternative but the fact something won't necessarily happen needs to be explicit. – GoatInTheMachine Feb 5 '18 at 16:20
  • @GoatInTheMachine I wholeheartedly disagree. Doing something or nothing is an implementation detail. A good name hides implementation detail behind an idea that ensures that the details can change yet will come as no surprise to someone who had previously only known the name. – candied_orange Feb 5 '18 at 16:25
12

You're trapped in a structural way of thinking. A name should abstract away implementation details. It shouldn't become a short hand for them.

 IfBirthdayBuyCake();

is a terrible name. It exposes implementation details as badly as if you wrote them here naked.

I would reach for a better name and a better abstraction.

celebrateHoliday();

Somewhere in it you'll likely find something like this

if ( birthday() ) {
    buyCake();
}

and maybe some other holidays. When you call it, it may do nothing at all. Which means when you call it you don't have to know if today is a holiday. That not knowing frees you from that detail. That's what a good name should do.

  • 2
    But what if celebrateHoliday() was called daily (figuratively)? I would be slightly confused as the name implies that every day is a holiday, and prefer rewriting to have an if(isHoliday()) check first. Now suppose the date is hard to retrieve and both methods need the result: then we're back at my original point. – aviator Feb 4 '18 at 23:47
  • 2
    Like I said, you're trapped in structural thinking. I'd have no trouble calling it daily. That's because it's a command not an event. You're not promised that it will do anything at all. – candied_orange Feb 5 '18 at 0:00
  • 3
    Viewing a method as a command rather than a promise is a good point. – aviator Feb 5 '18 at 9:56
  • 2
    @aviator thank you. Sometimes the postcondition is "well I tried". It goes against instinct. We want to dictate so we know what will happen. We want to have all the details. We end up drowning in them. Polymorphism and abstraction only work when you don't know the details. Learn to enjoy not knowing and not caring. – candied_orange Feb 5 '18 at 10:03
  • 3
    @CandiedOrange I must disagree - "celebrateHoliday()" would be a very bad name for a function that checks if there is a holiday to celebrate, and - if there is one - performs the necessary steps (e.g. buys cake). What does "celebrateHoliday()" appear to promise? That it celebrates Holidays. Period. There is no "if today is a holiday" condition in that name. – CharonX Feb 5 '18 at 12:53
2

I would advise against putting conditionals in your method names. If you want the calling code to read like a conditional, use a conditional:

if (cakeIsNeeded) buyCake();

Or, if ternary operators or short-circuits are your thing:

cake = cake == null ? buyCake() : cake;
cake = cake || buyCake();

Otherwise, you can either silently ignore repeat calls, use memoization, or throw exceptions in the method to deal with repeat calls — whatever feels most appropriate and least surprising for the particular method.

If you have a method name that feels like repeat calls would re-perform their action(s) but won't, one way to beat the "surprise" is to add an optional force, skipCache, or similar boolean parameter. (The name of the flag should be relevant to the method name and/or skip-logic.)

All that said, I'd tend to look for a verb that implies what the caller needs, rather than what the method does. In the example of cake, the caller wants the cake and doesn't care much where it comes from. It sounds to me like you just want to getCake() or findCake().

Both of those names communicate that Cake will be returned. They don't reveal to the caller how that Cake will be located. It could be purchased, taken from the counter, or made by magical elves. Those are implementation details.


One important caveat to all of this: These naming patterns tend to be highly idiomized. Refer to your language's internal libraries for examples of how they handle this. And talk to your team to decide on your own internal idioms.

  • Tried to keep this general to keep it shorter. But, if specific examples of when I'd personally ignore repeated calls, memoize, throw exceptions, etc ... I'll try to come up with some. ... Though, at the end of the day, the stress needs to be on following your language's and team's patterns and idioms. – svidgen Feb 5 '18 at 17:07
1

If there is a better alternative that still prevents double code execution, feel free to let me know :)

If preparatory calculations is really expensive and calculating it twice would really hurt the performance (so it is not a case of premature optimisation) I would still seperate condition and action into two methods because reading

if (shouldPerform()) doPerformAction()

is much more intituitive. To cope with the state calculation you can enclose the preparatory calculations into class state like this:

public class MyClass {
    private CalculationResult preparatoryCalculationResult = null;

    public boolean shouldPerform() {
        initIfNeccessary();
        return preparatoryCalculationResult.shouldPerform();
    }

    public void doPerformAction() {
        initIfNeccessary();
        preparatoryCalculationResult.doPerformAction();
    }

    private void initIfNeccessary() {
        if (preparatoryCalculationResult == null) {
            preparatoryCalculationResult = 42; // very expensive calculation ;-)
        }
    }

}
0

Reasonably common is "performActionIfNeeded" (a method that will cleverly figure out whether an action is needed and only perform it in that case) vs. "performActionIfWanted" (a method that will cleverly figure out that you want to perform an action and only perform it in that case).

  • 2
    I've never seen this variation. – Robert Harvey Feb 5 '18 at 3:32
  • 2
    @RobertHarvey From the mighty Spring Framework (Java) org.springframework.data.redis.cache.RedisCache#putIfAbsent I Knew I had seen this somewhere. – Laiv Feb 5 '18 at 8:10
  • @Laviv Java 8's Map interface has putIfAbsent as well. – pllee Feb 5 '18 at 19:23
  • 1
    @user949300 putIfAbsent in Java 8's Map has nothing to do with threading. Also if it did I fail to see how that matters for the question. – pllee Feb 6 '18 at 17:21
  • 1
    @RobertHarvey A few examples can be seen in Apple's UIKit framework, e.g. layoutIfNeeded(), updateConstraintsIfNeeded() and loadViewIfNeeded(). – Wolfgang Schreurs Feb 9 '18 at 8:31
0

Going on:

To clarify, it is important to the caller that the action may sometimes not be executed, otherwise it seems logical to me to use performAction().

It seems that the caller has some knowledge of what is going to happen. I think it is therefore warranted that the caller can decide whether or not the action should be performed.

An example:

A function should delete all files that match a pattern, but only if their total size exceeds some value. The scanning is expensive, but must be done in order to check the condition. The delete action requires knowledge of the scan in order to prevent scanning twice.

I would simply split this in two methods:

// Returns a ScanResult containing files matching a pattern
ScanResult performScan(Pattern pat);  

// Deletes a list of files
void delete(List<File> files);

The condition can now easily be externalized:

ScanResult result = obj.performScan(...)

if(result.totalSize() > 100 * 1024) {
    obj.delete(result.getFiles());
}
0

Part of the problem is the void return type of the original method. It would be better to return some kind of result struct object to the caller to indicate differing degrees of success. This gives the caller the information about what did or didn't happen, in case it needs to know to pass the message back to the UI or something similiar. It also quickly/clearly signals to the next coder to touch the code that the method called might not perform heavy operations with every call.

public class OperationResult
{
    public OperationState State { get; set; }
    public string OperationMessage { get; set; } // result codes, etc
    public enum OperationState { NoOperationPeformed, OperationSuccessful } // can have more if needed
}

You might return the whole class above, if you need the OperationMessage (or other data about the call), or just the enum OperationState if you just want to signal that the 'real' work of the function was skipped.

Its possible a fellow coder might assume from that void return that the method always does the same thing. Returning the above could help indicate through IDE intellisense that there's some other possibilities.

protected by gnat Feb 6 '18 at 13:56

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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