Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

This isn't a rare case and occurs with me often and I spent countless time trying to debug the code.

Latest example, this PHP code where I used $email for the parameter and for object as well.

private function _mail( $email )
{
    if( filter_var( $email, FILTER_VALIDATE_EMAIL ) ):
        $email = new Zend_Mail();
        $email->setSubject( $this->request->post('subject') )
            ->setBodyHtml( $this->request->post('message') )
            ->setFrom( $this->request->post('from') )
            ->addTo( $email )
            ->send();
        unset($email);
    endif;
}

It was throwing an error, strtr() expects parameter 1 to be an string object given and it was really frustrating to browse through zend libraries to see what obscure string statement was throwing this error.

How to avoid such mistakes?

share|improve this question
5  
someone will suggest hungarian I can feel it –  ratchet freak Jan 15 '13 at 14:12
1  
What IDE do you use? Static analysis should be able to pick up on these. –  StuperUser Jan 15 '13 at 14:13
1  
@StuperUser: I use PHPStorm. It does detect immediate assignment fallacies but sometimes it doesn't. –  Shubham Jan 15 '13 at 14:22
4  
That's the first time in my life that I ever seen endif used like that!! I mean yeah that was popular when PHP and HTML were mixed together, but endif in a function, that's a first. –  Songo Jan 15 '13 at 15:13
1  
Honestly, all the 3 answers so far suck. Variable naming doesn't solve this problem in general and puts the blame on the wrong place. Isn't there any linter out there that gives a warning if you assign to a function parameter? –  hugomg Jan 16 '13 at 2:15
show 3 more comments

5 Answers 5

up vote 13 down vote accepted

One good aspect of modern programming languages is that you can have large variable names. In Object oriented programming, which you are utilising, Java conventions are quite popular.

So it would be better to have a descriptive name for the Zend_Mail object, such as

$zendMail = new Zend_Mail();

In the beginning it will consume a bit more time until the brain shifts making it a habit, but that time will have a huge return of investment and stress from having to debug similar cases.

share|improve this answer
7  
In addition, change the parameter name to something like $toAddress. $email could mean so many things. –  Chris Jan 15 '13 at 17:02
1  
This is a very general solution: if your variable names describe exactly what they contain, the chance of collisions is very low. –  Joachim Sauer Jan 17 '13 at 8:47
add comment

In PHP I prefix function/method paramaters with 'in' - and don't use any variable starting with 'in' within the body. It's not ideal, but it works.

Dimitris point about more descriptive names is also helpful.

share|improve this answer
add comment

I think that you have two different concept here. You have the email address of a recipient and an email message that will be sent to the recipient. So why not :

private function _mail( $recipient )
{
    if( filter_var( $recipient, FILTER_VALIDATE_EMAIL ) ):
        $mail = new Zend_Mail();
        $mail->setSubject( $this->request->post('subject') )
            ->setBodyHtml( $this->request->post('message') )
            ->setFrom( $this->request->post('from') )
            ->addTo( $recipient )
            ->send();
    endif;
}
share|improve this answer
add comment

This isn't a rare case and occurs with me often and I spent countless time trying to debug the code.

Just that much tells me two things:

  1. If it hurts, stop doing it. You say that this problem "occurs with me often," which means that you need to make a greater effort to avoid this kind of problem. Don't try to use two variables with the same name, ever, even when you're sure that it'll work. There are two possibilities: a) you're right and it works, but your code is a lot more difficult to understand than it should be; or b) you're wrong and, well, you've seen what happens when you're wrong.

  2. Learn from experience. If you keep writing the same kind of bug, you should by now be pretty darn good at recognizing the symptoms and finding the problem. Knowing that you have a propensity for creating a certain class of bug, you should constantly be on the lookout for indications that you've done it yet again. Use assertions to check that a variable contains the type of data that you expect. Question your assumptions. Write some unit tests to test individual components.

share|improve this answer
add comment

My IDE does that for me. Eclipse has "templates" for code-autocomplete (STRG+Space) I told my Eclipse to always add a prefix to method parameters.

For example your method:

private function _mail( $recipient )

would automatically be auto-completed with $pRecipient.

Short Rule: Always prefix method parameters, to avoid variable clashes inside the method body.

sidenote: think about using interfaces instead ob objects as method parameters.

share|improve this answer
    
I knew someone would suggest hungarian ;) –  ratchet freak Mar 1 '13 at 12:41
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.