开发者

Best practice: Validate conditions for method calls?

I guess in almost every programm sometimes methods don't need to be called all the time but under specific conditions only. It it very easy to check if a method must be called. A simple if-statment can do the trick.

if (value == true)
{
    DoSomething();
}

But if you have many conditions the validation can get complicated and the code gets longer and longer. So I wrote code with the method called every time and the method itself will check and validate if her code needs to be executed.

DoSomething(value);

... then ...

public void DoSomething(bool value)
{
    if (value == true)
    {
    // Do Something here ...
    }
}

Now I have two ways of doing things. I am not exactly su开发者_如何学Pythonre which way is the right way. Or maybe there is even another option?


Clean Code — A Handbook of Agile Software Craftsmanship promotes not to write methods accepting a single boolean parameter because each method should do one thing and one thing only. If a method takes a boolean parameter to decide what to do, it automatically does two things: deciding what to do and actually doing something. The method should be refactored into two separate methods doing something and a single method deciding which of the two methods to call.

Furthermore, evaluating a boolean value using value == true is redundant and unnecessary. The value itself represents a boolean state (true / false) and does not need to be compared to true again. That said, the best practice is using if (value) instead of if (value == true) (or if ((value == true) == true; this seems idiotic but does not differ much from the approach of if (value == true)).


I find the answer to this question to be fairly obvious - unless I'm missing something. Adapt to each situation. The called function should do what it's intended to do. If its intention is to work on some set of arguments, by all means do the checking inside the function. If you plan to call the function conditionally, do the checking outside. Moving the check inside just so you can save some extra verification is not a good idea I think, since others might want to call your function and not know whether it actually works given their parameters. I say, unless checking inside is imperative, leave the checking outside.

EDIT: I just re-read your question... You basically have:

void foo(bool actuallyExecuteFoo)
{
    ////
}

Really? REALLY?


But if you have many conditions the validation can get complicated and the code gets longer and longer.

If the validation is complicated, it means that the logic underneath is complicated. Expect your code to be as complicated as your logic - it has to be somewhere out there, right? Just think how to write it in a clean way. And the clean way is not always the shortest way.


I recommend this variant:

if (value == true)
{
    DoSomething();
}

Why? Because:

  • the code calling DoSomething is then more clear (*), as it explicitly shows when the logic of DoSomething should be executed and when not,
  • DoSomething itself depends on less parameters (which makes it more generic and reusable).

*) Yes, "more clear" actually means "longer" here, but it also means "explicit" and "self-documenting". The shorter variant actually tries to hide some logic, which makes the code less clear.


Check out this for C# not sure why you need C++ :)


If the method can throw a Argument/ArgumentNull Exception, then you should validate it before calling the method. Also, Microsofts Code Contracts, will tell you if you need to validate the input before calling the method, for any code using contracts (basically assertions for static analysis).

The general rule is not to validate the input more than necessary. If something isn't valid, you should throw a exception (C#), or return a error (C++). Not executing the code due to a invalid input, without telling why, makes it near-impossible for the next-developer to figure out what the problem is.


I would recommend the second way, but I have some remarks:

  1. You do not need to check if (value == true), just check if (value) instead.

  2. Return earlier, what I mean if (!value) { return; }


Second way will take more execution time, though code will look better. Why don't you use macro?

#define DOSOMETHING(value)  if (value) {DoSomething();}

Replace all

if (value == true) {DoSomething(); }  

with macro DOSOMETHING(value) Your purpose will be solved and code will look better

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜