views:

328

answers:

8

I sometimes use temporary variables to shorten the identifiers:

private function doSomething() {
    $db = $this->currentDatabase;
    $db->callMethod1();
    $db->callMethod2();
    $db->callMethod3();
    $db->...
}

Although this is a PHP example, I'm asking in general:

Is this bad practice? Are there any drawbacks?

+16  A: 

This example is perfectly fine, since you are using it in functions/methods.

The variable will be unset right after the method/function ends - so there's not much of a memory leak or what.

Also by doing this, you "sort of" implemented DRY - don't repeat yourself.

Why write so many $this->currentDatabase when you can write $db. And what if you have to change $this->currentDatabase to some other values?

thephpdeveloper
You will have a memory leak if you'll store that reference somewhere, and forget about it.
Geo
@Geo - if you're storing the reference somewhere, you'd have been doing so *anyway*. This question is clearly about local variables as aliases of a sort. If you "accidentally" store local variables globally, you're going to run into big problems anyway.
Andrzej Doyle
Even so, the variable will also be unset right after the script execution ends.
thephpdeveloper
@thephpdeveloper, that isn't what DRY means. DRY is about not defining an entity in more than one place, like having a constant defined in a configuration file AND in your source code (instead of reading it from the configuration file). The reason not to repeat oneself is to avoid the errors that crop up from having to do double maintenance (such as changing a value in the config file, but forgetting (or not knowing) to change the source code too).
Craig Trader
+7  A: 

Actually, you're not trying to avoid typing (otherwise, you'd use a completion mechanism in your editor), but you're just making your function more readable (by using "abbreviations") which is a good thing.

Drawbacks will show up when you start doing this to avoid typing (and sacrifice readability)

Gyom
+3  A: 

In general :

  • Both $db as $this->currentDatabase point to exactly the same object.
  • The little space allocated for $db is freed (or elligeable for garbage collection) when the function ends

so I'd say : no, it's not bad practice.

Peter
+1  A: 

If you do this carefully it is absolutely fine. As long as you only use a few of this variables in a small amount of code and inside of small functions I think this is ok.

If you have a lot of this variables and they are badly named like i,j,l and f in the same function the understandability of your code will suffer. If this is the case I would rather type a little bit more then have not understandable code. This is one reason a good IDE has automatic code completion.

Janusz
+1  A: 

No, I think, this is ok. Often performance if not as critical as clean readable code.

Also, you are trading memory a small allocation hit on the stack for faster method calls by avoiding extra dereferencing.

JeffV
+3  A: 

I seem to remember that Steve McConnell recommends against using temporary variables in "Code Complete". At the risk of committing heresy, I have to disagree. I prefer the additional readability introduced. I also find myself adding them to aid single-step debugging, then seeing no reason to remove them.

MikeJ-UK
I agree with you and disagree with McConnell here. I also prefer using local variables over multiple-line-method calls for example. It also makes debugging easier, because you can check the value of each variable.
panschk
+3  A: 

It depends what is the contract on $this->currentDatabase. Can it change at any time, after any method call? If it changes, are you supposed to keep on using the object you did when you made your first db call, or are you supposed to always us the current value? This dictates if you must always use $this->currentDatabase, or if you must always store it in a variable before using.

So, strictly speaking, this is not a style question at all.

But, assuming the member is never changed during function calls such as this, it makes no difference. I'd say storing it in a variable is slightly better, as it is easier to read and avoids a member access on an object at every operation. The compiler may optimize it away if it's good, but in many languages such optimizations are very difficult - and accessing a local variable is almost invariably faster than accessing a member of an object.

Nakedible
+1  A: 

I don't think there is a performance penalty if you use the original variable instead of skipping the first dereference ($this->currentDatabase).

However, as readability is much improved using the abbreviation, go for it!

Of course it also will depend on your team's coding conventions.

Pablo Rodriguez
If getting $this->CurrentDatabase involves a function call (I don't know any PHP so I can't tell), it should be FASTER to use a temporary variable, as the function is only called once. Nut bear in mind Nakedible's answer.
Gerry