3

I was wondering if it is good practice to declare variables just to make code (such as a long condition in an if statement) more human readable or if this wastes resources and using detailed comments is better?

Here's a simple example.

//declare variables for if statement only
int price = getProductionCost() + getTax() + getPurchaseMethod() + etc + etc;
int shipping = getLocation() + getShippingType() + etc;

if (price + shipping > 1000)
{
    // Is this better practice using variables?
}

// if price + shipping > 1000
if (getProductionCost() + getTax() + getPurchaseMethod() + etc + etc + getLocation() + getShippingType() + etc > 1000)
{
    // or is this option better practice with commenting and no variables?
}

I am also aware that there is a risk of the variable being modified in the same method which is a disadvantage. I tried to look for best practices on this but couldn't find anything and wasn't sure what to search for. Thank you.

justlearning
  • 165
  • 1
  • 6
  • 6
    "best practice" questions are off-topic for stackoverflow since it encourages cargo-cult programming rather than actually thinking about solutions to a problem. But, I usually use descriptive variable names, unless it's just a loop counter or an index in an array. – user1666620 Sep 14 '15 at 11:08
  • 1
    Thank you for your answer and for educating me on forum topics. I won't post any more on best practices! – justlearning Sep 14 '15 at 11:16
  • @user1666620 , I think it is not about cargo cult programming. Regarding question. IMHO you can use following as a simple rule - code line should not go beyong screen generally. Length of line depends on average screen resoution in your team. In exact your example, separate method GetTotatlPrice - seems reasonable. – Timur Mannapov Sep 14 '15 at 11:16
  • I only declare them if I do some magic with it after the if statement (inside or outside). You could also make a method GetPrice or so. – kevintjuh93 Sep 14 '15 at 11:17
  • @TimurMannapov Thanks for your opinion. Regarding my specific example, this was just to highlight a general point not part of my actual program :) – justlearning Sep 14 '15 at 11:22

5 Answers5

4

I think this way is better:

if (price + shipping > 1000)
{
    // Is this better practice using variables?
}

But you should write your methods names with upper-case first symbol:

// Like this
GetProductionCost()

// Not like this
getProductionCost()
Volodymyr Sichka
  • 531
  • 4
  • 10
3

Generelly if you add a comment you can also just simply make your code self-exlenatory so it won´t need the comment. Having said this you should always consider to make a variable self-speeking enhancing UNDERSTANDING of your code.

However maintaining a long complicated expression may be teribly awfull. Thus you should also for the sake of maintainablilty consider to split your code into smaller, easier to read parts.

MakePeaceGreatAgain
  • 35,491
  • 6
  • 60
  • 111
3

The danger of using comments is that they can get out of date. Consider the situation where youchange the threshold to 1100, but forget to change the comment:

// if price + shipping > 1000
if (getProductionCost() + getTax() + getPurchaseMethod() + etc + etc + getLocation() + getShippingType() + etc > 1100)
{
    //
}

Someone looking at the code now doesn't know whether the mistake is in the comment, or the code.

For this reason, it's better to favour easy-to-read code over difficult to read code with comments.

You could further enhance the code by splitting off the calculations into their own methods, rather than using local variables and use a constant for 1000:

public void SomeMethod()
{
    if (CalculatePrice() + CalculateShipping() > CostThreshold)
    {
        // do something
    }
}

...

private static int CalculatePrice() => getProductionCost() + getTax() + getPurchaseMethod() + etc + etc;
private static int CalculateShipping() => getLocation() + getShippingType() + etc;
David Arno
  • 42,717
  • 16
  • 86
  • 131
  • note that the special syntax there is only available in C#-6.0 – D. Ben Knoble Sep 14 '15 at 13:13
  • @BenKnoble, now that VS2015 has been released and C# 6 is the current version of the language, is it necessary to still explain its use? – David Arno Sep 14 '15 at 13:15
  • Eh. Maybe not everyone is upgraded. I'm with you, butttt.... for example, Xamarin will not recognize c#6 usages (like the $"" and nameof() ) but it compiles fine. – D. Ben Knoble Sep 14 '15 at 14:21
  • @BenKnoble. Fair point. I shall revert to stating that I'm using C#6, when I do. Thanks. – David Arno Sep 14 '15 at 14:39
  • @DavidArno Thank you for your input. Useful to know about updating comments with code. Regarding this example, it was just something I thought of before writing the question to give it some context, not a real example :) but this information is invaluable to me. – justlearning Sep 14 '15 at 18:21
1

I'd go for the variables to make the if-statement look more clean.

Code is read more often than it is written, but there is nothing preventing you from writing comments too. Just don't overdo it!

sweerpotato
  • 470
  • 2
  • 11
  • 22
  • Thanks. I am just learning and have been using variables to make the code clean for me to read; however I want to use best practices for the future too and was unsure if variables increased memory consumption and simply for the fact that the values can be modified by other code, makes it more vulnerable to problems – justlearning Sep 14 '15 at 11:14
0

An if condition will always come in a method block & creating a local scope variable is fine. About the risk that you have mentioned : I don't think it is a risk because as per the "best practices" we do not create a very long methods, and in short methods we do not mistake with what variable need not modified. Additionally you can write a comment on the variable, so that other developer who intend to modify the code will know what/why he is doing. PS: Adding comment is always good, put them wherever you think is a place to comment.

Shivendra
  • 159
  • 1
  • 7