Cleaner Coding

[muncherw]

While writing a script tonight I realized I could combine a few steps together to make fewer lines of code. It got me wondering if one way was better than another or if it's mostly just stylistic choice. I've created four simplified examples to illustrate my question. These all give the same result, which it to display the shortened name of the month in lower case.

Does one of these work faster than another? ie Does it take longer to execute 4 lines of simple code or one line that's slightly more complex? What if that one line was significantly more complex? Or is somewhere in the middle better? I understand that the difference is going to be non-exsistant to the naked eye but I would expect in something with thousands of lines of code those differences could total up to seconds or more.

Which is easier to read? If you picked up a project where you had to look at someone else's code and finish it for them which of these would make it easier to understand what the code did faster?

Is one way preferable to another? Which is more elegant, more efficient, or is it all pretty much the same? I'm asking on a broad scale not confined to the code below. I just thought that they might help make the question clearer. I'd like to use the best technique when creating my own scripts.

Four steps:

$month = @mon
    $month = _DateToMonth($month, 1)
    $month = StringLower ( $month )
    MsgBox(0, "Month", $month)

Probably the way I would normally do it:

$month = _DateToMonth(@mon, 1)
    $month = StringLower ( $month )
    MsgBox(0, "Month", $month)

A little more compact:

$month = _DateToMonth(@mon, 1)
    MsgBox(0, "Month", StringLower ( $month ))

All four steps in one line of code:

MsgBox(0, "Month", StringLower(_DateToMonth(@mon, 1)))

[Distrophy]

It is usually best practice to keep everything split up as much as possible.

like you did here:

$month = _DateToMonth(@mon, 1)
    $month = StringLower ( $month )
    MsgBox(0, "Month", $month)

It shoudln't take any longer for your program to process each line, it is just so you can see what you have done more easily.

The reason you would want to keep each line simple is because if youve written a large amount of code, should something ever break in the future, you will spend hours just trying to figure out what it is youve done in your code from before.

Quick and dirty coding is Okay if you need something very small, but if you plan on using the code for a while you should use descriptive variable names and clean broken-down code.

[FaT3oYCG]

it is good practice to understand your code but not to necesarily spit it up, the compiler should do most of the work for you if you do split you code up as it basically replaces any variables with their actual values, but it is good practice to go over the code yourself and do what the compiler does, if you can turn four lines into one then do that, if you find that you need something a lot then use a variable and simply replace it with the find an replace tool of your editor at the end. you obviously wouldnt do that for a variable that has a changing value that is calculated in the program though, such as a level counter or such.

i take a computing class at college and we were advised the above so im pretty sure its correct and to the point.

thanks

Mr. C

[Distrophy]

I dont know what college you went to, but the rest of the world has defined what are considered "best coding practices" and a simple google search will give you hundreds of pages to see for yourself.

[muncherw]

Any one else have feelings on this? I'm inclined to do the slightly longer code because it's a littler easier to follow but I would like more opinions if anyone has them.

[MilesAhead]

Any one else have feelings on this? I'm inclined to do the slightly longer code because it's a littler easier to follow but I would like more opinions if anyone has them.

I used to program a lot in C and C++ where it's cool to use "side effects".

As example assignment also gives off the value assigned so you can use the

value as a boolean result. For instance if the function returns a pointer where

0 is considered NULL that could be tested as a fail. Any non 0 is considered

"success" so you see a lot of code of the type

if(ptr = SomeFunc(inner_func(some_param, another_param),yet_another_func(anotherparam)))

{

do stuff

}

;parens prolly ain't balanced but you get the idea

Only trouble is when you debug you're going to get a line of code that causes the error. If you call

twenty functions in that one line then you have to step through to see where the fault is.

But if you break each call out to its own line, it's obvious which function call didn't work.

Though it looks more pedestrian I got into the habit of step ladder test for result at each

call or use exceptions where you throw a different code in each called function so that

your catch statement can discriminate(probably one of the main reasons they came up

with exception handling so as to avoid step ladder if statements.)

So I guess the short way to say it is, if you want to tightly group your code use exceptions or else

you'll (potentially)be lost during the debugging.

Edited October 24, 2008 by MilesAhead

[Richard Robertson]

It's best to code in a way that is understandable. Sometimes that means to put everything on one line. Sometimes that means a 10 line function instead of a complex equation.

Just look at it and if you can't figure out what it does in 10 seconds, you should break the line down.

Edited October 24, 2008 by Richard Robertson

[muncherw]

I'm inclined to agree that being able to eyeball the code and knowing what it does at a glance is preferable. Most of you are saying much the same thing but I like the differing reasons behind them. Thanks a lot everyone. I know this isn't sctrictly a question on how a particular piece of code is or isn't working so I appreciate your patience and leniency.