doSomethingIfCondition(). Is it good naming? [closed]

Suppose, I have some method that performs some job. In this example, it’s going to be void, but it doesn’t matter

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public void doSomething() {
// doing something
}
</code>
<code>public void doSomething() { // doing something } </code>
public void doSomething() {
    // doing something
}

But then, let’s imagine, I got my team mates complaining that my method, on occasion, doesn’t actually do “something”. It’s because it includes some condition check. It could be a state check or input validation, but since my sample method doesn’t take any arguments, let’s assume it’s the former

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public void doSomething() {
if (isCondition()) {
/*
I guess I could ask a separate question
on whether doDoSomething() is good naming
*/
doDoSomething()
}
}
</code>
<code>public void doSomething() { if (isCondition()) { /* I guess I could ask a separate question on whether doDoSomething() is good naming */ doDoSomething() } } </code>
public void doSomething() {
    if (isCondition()) {
        /* 
           I guess I could ask a separate question 
           on whether doDoSomething() is good naming
        */
        doDoSomething()
    }
}

I have two options:

  1. Document it
  2. Incorporate that information in the method’s name

I recall from Bob Martin’s Clean Code that comments should always be seen as a failure — a failure to write code that explains itself. Obviously, I don’t want to fail (I want to win, win, win). So I opt for the latter option

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public void doSomethingIfCondition()
</code>
<code>public void doSomethingIfCondition() </code>
public void doSomethingIfCondition()

Then, another change. For whatever reason, I now have to check one extra condition. Since I don’t want my method to lie (lying methods is a big no-no in clean code), I now have to change the method name again

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public void doSomethingIfConditionOneOrConditionTwo()
</code>
<code>public void doSomethingIfConditionOneOrConditionTwo() </code>
public void doSomethingIfConditionOneOrConditionTwo()

That’s ugly. But suppose I can somehow group those conditions semantically

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public void doSomethingIfSuperCondition() {
if (isSuperCondition()) {
doDoSomething()
}
}
</code>
<code>public void doSomethingIfSuperCondition() { if (isSuperCondition()) { doDoSomething() } } </code>
public void doSomethingIfSuperCondition() {
    if (isSuperCondition()) {
        doDoSomething()
    }
}
Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>private boolean isSuperCondition() {
return isConditionOne() && isConditionTwo();
}
</code>
<code>private boolean isSuperCondition() { return isConditionOne() && isConditionTwo(); } </code>
private boolean isSuperCondition() {
    return isConditionOne() && isConditionTwo();
}

A spec overhaul. Now, neither “condition one” nor “condition two” matter one bit. It’s “condition three” that really does. But here’s the problem: my method became quite popular. There are a ton of clients, and it’s not feasible to fix them. Or, perhaps, I don’t even control all of my clients anymore. It became opensource or something. So the best I can come up with is this

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>/**
@deprecated use {@link #doSomethingIfConditionThree} instead
*/
public void doSomethingIfSuperCondition() {
if (isSuperCondition()) {
doDoSomething()
}
}
</code>
<code>/** @deprecated use {@link #doSomethingIfConditionThree} instead */ public void doSomethingIfSuperCondition() { if (isSuperCondition()) { doDoSomething() } } </code>
/**
  @deprecated use {@link #doSomethingIfConditionThree} instead
*/
public void doSomethingIfSuperCondition() {
    if (isSuperCondition()) {
        doDoSomething()
    }
}
Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public void doSomethingIfConditionThree() {
if (isConditionThree()) {
doDoSomething()
}
}
</code>
<code>public void doSomethingIfConditionThree() { if (isConditionThree()) { doDoSomething() } } </code>
public void doSomethingIfConditionThree() {
    if (isConditionThree()) {
        doDoSomething()
    }
}

I can keep making this story up. The point is, I start to doubt that naming convention. I want to write clean, but trying to do so, in my fictional example, resulted in a pretty nonsensical code. I am not at all sure it’s what Bob Martin intended

So, what do you think?

21

Code is full of conditionals. You can’t specify the conditionals in all the symbol names!

If you find yourself tempted to include a condition in the name, it’s possible your name is too low-level. There is probably a higher-level way of looking at the activity that would result in a name without the condition.

Instead of

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>void CreateDirectoryIfItDoesntExist(string path)
</code>
<code>void CreateDirectoryIfItDoesntExist(string path) </code>
void CreateDirectoryIfItDoesntExist(string path)

Try

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>void EnsureDirectoryExists(string path)
</code>
<code>void EnsureDirectoryExists(string path) </code>
void EnsureDirectoryExists(string path)

Or instead of

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>void StartIfReady();
</code>
<code>void StartIfReady(); </code>
void StartIfReady();

Try

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>void TryStart();
</code>
<code>void TryStart(); </code>
void TryStart();

Or instead of:

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>void UseRedIfEmergercy();
</code>
<code>void UseRedIfEmergercy(); </code>
void UseRedIfEmergercy();

Try:

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>void ApplyPriorityColorLevel();
</code>
<code>void ApplyPriorityColorLevel(); </code>
void ApplyPriorityColorLevel();    

Or instead of:

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>void WriteToLogIfDebugEnabled(string message);
</code>
<code>void WriteToLogIfDebugEnabled(string message); </code>
void WriteToLogIfDebugEnabled(string message);

Try:

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>void LogDebug(string message);
</code>
<code>void LogDebug(string message); </code>
void LogDebug(string message);

1

Here’s the appropriate prefix:

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>bob.jump();
</code>
<code>bob.jump(); </code>
bob.jump();

What bob does when told to jump is up to bob. Bob could jump. Bob could ask you “how high?”. Bob could ignore you completely. Jump isn’t about what happens. Jump is a message you sent to bob. What bob does with it is up to bob.

Whenever you find yourself mutilating a name by filling it with implementation details understand you are harming abstraction. A name should tell me why to call it. Not what it does. Telling you what it does is not a names job. Name things that way and you wont have to rename them when you refactor.

Remember, a name is a comment. Comments should tell you why the code exists. Not what it does.


This answer has received a mixed reception and I can see it deserves some revision. Sorry it has taken so long. Family has its needs.

Let me ask this*:

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>ConfiguredWithNoFileSystem.EnsureDirectoryExists(somePath);
SystemThatNeverStops.TryStart();
BlackAndWhiteDisplay.ApplyPriorityColorLevel();
</code>
<code>ConfiguredWithNoFileSystem.EnsureDirectoryExists(somePath); SystemThatNeverStops.TryStart(); BlackAndWhiteDisplay.ApplyPriorityColorLevel(); </code>
ConfiguredWithNoFileSystem.EnsureDirectoryExists(somePath);
SystemThatNeverStops.TryStart();
BlackAndWhiteDisplay.ApplyPriorityColorLevel();

Isn’t the Null Object Pattern something that could be useful here? Isn’t silently doing nothing ok when that’s what is really needed?

* I didn’t include LogDebug(); in this list because silently doing nothing when so configured is something people have already accepted it doing.

I absolutely agree with John Wu’s point that a higher level of abstraction can free you of details that might clutter a name. He gives many good examples that communicate the expected post condition of the sunny day path. But permit me one nit pick. TryStart() is not a higher level of abstraction.

It is only announcing what you should already know. That this method might not do what you ask it to do depending on how it is configured. You should expect that even if it doesn’t throw, even if it doesn’t return an error, even if it doesn’t report the issue thru some output port, it might do nothing, and silently ignore you. There is no method that could never end up needing to be configured to behave in this way.

The reason I feel that way is because the most recent knowledge always wins. Maybe in the past we were sure this will never be needed. Then time passes and it turns out we were wrong. Our job isn’t to predict the future. It’s to design systems that make it easy to deal with it.

I’ve taught this lesson for years, and I told a story that illustrates it in the middle of this somewhat old answer of mine.

The hard principle behind this idea is that reality doesn’t care what you think. This is why we don’t use composite natural keys in a live database. You may think combining phone number and social security will always ensure a unique id but all anyone has to do is add one more row and suddenly it doesn’t work anymore. No, do that with dead data that you’re trying to structure. And soon as you can make a unique key that isn’t used for anything else besides being a unique key. That way reality can’t mess with you.

26

The method’s job is to do something.
If that something is only done based on condition(s) known only to the method, then so be it. The caller does not need to know or care about them.

If they did, then they would be testing the preconditions themselves …

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>if( condition )
doSomething();
</code>
<code>if( condition ) doSomething(); </code>
if( condition ) 
   doSomething(); 

… which makes for even messier code or passing the conditions as arguments to the method for evaluation there.

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public void doSomething( bool condition )
</code>
<code>public void doSomething( bool condition ) </code>
public void doSomething( bool condition ) 

(Incidentally, this approach would make the addition of conditions 2 and 3 clearer, because you’d need a new method for each).

It seems to me that the method is working just fine.
The problem is that the Developers aren’t aware of these internal conditions. The way to communicate such things to Developers is through the Documentation.

1

An appropriate name is

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>trySometing()
</code>
<code>trySometing() </code>
trySometing()

Method should inform client, that it does not guarantee the named side-effect, but it should not reveal any implementation details.

This pattern is usually associated with exception handling, but IMO it is useful without those too.

7

You tend to find these naming and placement difficulties arise from a somewhat awkward conceptualisation in the first place, or from an unsound division of labour between the person who writes the method and the person who calls it, which leads to the callers using the method with an unsound understanding of what it does.

With these kind of methods which self-check, and which silently and unexpectedly become a no-op under certain circumstances, what it tends to imply is that a condition should be checked at the call site before the main call occurs and that the caller should be in charge of that check.

The reason there is a deviation from this pattern and there is an attempt to fold the check into the called method, is either because the check has to be made annoyingly frequently at different call sites, or because developers keep forgetting to make the check and the code runs incorrectly for that reason.

The problem is, having folded a silent check into the method, your developers are expressing surprise about the occasional no-op behaviour. That suggests they should be rehearsing the check themselves explicitly in code at the call site, because they (apparently) need to know that the check is occurring and are tending to forget that it does.

If the checking code is itself complicated, then there is nothing wrong with you providing that as a checking method separately from the main method, but the caller still has to go through the pattern of making the check then deciding whether to make the main call, and therefore cannot forget the possibility that the main call (and the work associated with it) does not occur.

If necessary, the check can be performed redundantly inside the method, throwing an invalid op if the call has occurred in the wrong circumstances.

However, if you stick with the existing code structure, then DoSomethingIfCondition is as good a name as any.

4

Here you use naming convention like

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public void Profileverification();
</code>
<code>public void Profileverification(); </code>
public void Profileverification();

or

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public void UpdateSessionIfExpire();
</code>
<code>public void UpdateSessionIfExpire(); </code>
public void UpdateSessionIfExpire();

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public void CreateDirectoryIfNotExist();
</code>
<code>public void CreateDirectoryIfNotExist(); </code>
public void CreateDirectoryIfNotExist();
Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public chekIfSessionExpire();
Or
public void IsSessionExpire();
</code>
<code>public chekIfSessionExpire(); Or public void IsSessionExpire(); </code>
public chekIfSessionExpire();

Or 

public void IsSessionExpire();

New contributor

n_dev is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.

Trang chủ Giới thiệu Sinh nhật bé trai Sinh nhật bé gái Tổ chức sự kiện Biểu diễn giải trí Dịch vụ khác Trang trí tiệc cưới Tổ chức khai trương Tư vấn dịch vụ Thư viện ảnh Tin tức - sự kiện Liên hệ Chú hề sinh nhật Trang trí YEAR END PARTY công ty Trang trí tất niên cuối năm Trang trí tất niên xu hướng mới nhất Trang trí sinh nhật bé trai Hải Đăng Trang trí sinh nhật bé Khánh Vân Trang trí sinh nhật Bích Ngân Trang trí sinh nhật bé Thanh Trang Thuê ông già Noel phát quà Biểu diễn xiếc khỉ Xiếc quay đĩa Dịch vụ tổ chức sự kiện 5 sao Thông tin về chúng tôi Dịch vụ sinh nhật bé trai Dịch vụ sinh nhật bé gái Sự kiện trọn gói Các tiết mục giải trí Dịch vụ bổ trợ Tiệc cưới sang trọng Dịch vụ khai trương Tư vấn tổ chức sự kiện Hình ảnh sự kiện Cập nhật tin tức Liên hệ ngay Thuê chú hề chuyên nghiệp Tiệc tất niên cho công ty Trang trí tiệc cuối năm Tiệc tất niên độc đáo Sinh nhật bé Hải Đăng Sinh nhật đáng yêu bé Khánh Vân Sinh nhật sang trọng Bích Ngân Tiệc sinh nhật bé Thanh Trang Dịch vụ ông già Noel Xiếc thú vui nhộn Biểu diễn xiếc quay đĩa Dịch vụ tổ chức tiệc uy tín Khám phá dịch vụ của chúng tôi Tiệc sinh nhật cho bé trai Trang trí tiệc cho bé gái Gói sự kiện chuyên nghiệp Chương trình giải trí hấp dẫn Dịch vụ hỗ trợ sự kiện Trang trí tiệc cưới đẹp Khởi đầu thành công với khai trương Chuyên gia tư vấn sự kiện Xem ảnh các sự kiện đẹp Tin mới về sự kiện Kết nối với đội ngũ chuyên gia Chú hề vui nhộn cho tiệc sinh nhật Ý tưởng tiệc cuối năm Tất niên độc đáo Trang trí tiệc hiện đại Tổ chức sinh nhật cho Hải Đăng Sinh nhật độc quyền Khánh Vân Phong cách tiệc Bích Ngân Trang trí tiệc bé Thanh Trang Thuê dịch vụ ông già Noel chuyên nghiệp Xem xiếc khỉ đặc sắc Xiếc quay đĩa thú vị
Trang chủ Giới thiệu Sinh nhật bé trai Sinh nhật bé gái Tổ chức sự kiện Biểu diễn giải trí Dịch vụ khác Trang trí tiệc cưới Tổ chức khai trương Tư vấn dịch vụ Thư viện ảnh Tin tức - sự kiện Liên hệ Chú hề sinh nhật Trang trí YEAR END PARTY công ty Trang trí tất niên cuối năm Trang trí tất niên xu hướng mới nhất Trang trí sinh nhật bé trai Hải Đăng Trang trí sinh nhật bé Khánh Vân Trang trí sinh nhật Bích Ngân Trang trí sinh nhật bé Thanh Trang Thuê ông già Noel phát quà Biểu diễn xiếc khỉ Xiếc quay đĩa
Thiết kế website Thiết kế website Thiết kế website Cách kháng tài khoản quảng cáo Mua bán Fanpage Facebook Dịch vụ SEO Tổ chức sinh nhật