Bad Habit? Extra variables inside methods

[ChadNomad]

Hi all,

 

I am nitpicking here but I have wondered what the negatives are of assigning local variables to existing class variables are. To make more sense here is an example:

<?php
class myClass {
private $_something;

public $config = array(
	'setting' => 123,
);

public function __construct(array $config)
{
	$this->config = $config;
}

public function go()
{
	/*
	 * Is this considered bad? I know an extra variable is being made
	 * but I find it tedious constantly using $this, static, self etc
	 */
	$something = $this->_something;

	if($something)
		...

	/*
	 * Another thing I've found myself doing is typecasting arrays
	 * to objects to save time typing. Is this bad?
	 */
	 $config = (object) $config;

	 if($config->setting === 123)
	 	...
}
}

 

I work alone mostly and have just started to become curious about all the bad habits I have.

 

Thanks

[KevinM1]

The problem with what you're doing is that, in addition to your changes being irregular and useless, you're making your code harder to read.  You're not gaining anything but a few keystrokes, and that's at the expense of clarity.

[smoseley]

Nothing wrong with setting class params with a constructor param of the same name (e.g. $this->foo = $foo).

 

However, when accessing class params later, just reference $this->foo - no need to set another local variable in the function.  It unnecessarily uses some processor time and memory.  Quicker and better to read from $this. 

 

If you want to temporarily modify the param for output, however, you should copy it, e.g.

 

function getSomethingWithCoolAlgorithmApplied($foo) {

    $something = $this->_something;

    for ($i = 0; $i < $foo; $i++) {

      $something *= rand(1,9);

    }

    return $something;

}

[ChadNomad]

The problem with what you're doing is that, in addition to your changes being irregular and useless, you're making your code harder to read.  You're not gaining anything but a few keystrokes, and that's at the expense of clarity.

 

I though this was the case. As its my own code I probably don't realise the inconsistencies I'm making for everyone.

 

Nothing wrong with setting class params with a constructor param of the same name (e.g. $this->foo = $foo).

 

However, when accessing class params later, just reference $this->foo - no need to set another local variable in the function.  It unnecessarily uses some processor time and memory.  Quicker and better to read from $this. 

 

If you want to temporarily modify the param for output, however, you should copy it, e.g.

 

function getSomethingWithCoolAlgorithmApplied($foo) {

    $something = $this->_something;

    for ($i = 0; $i < $foo; $i++) {

      $something *= rand(1,9);

    }

    return $something;

}

 

Thanks. I think its reaffirmed my suspicion that its all too unnecessary.

 

Thanks guys

[ignace]

If you are going to typecast to an object then do so in the constructor (or use an ArrayObject) and not in every method.

[smoseley]

Oh, missed the casting bit.  Yes, it's bad.  Casting to an object adds unnecessary overhead.  Further, PHP's arrays are extremely powerful.  They are a collection / hash, iterator, and array all in one, and there are so many built-in functions for them, you're shooting yourself in the foot by switching to an object.