Click to See Complete Forum and Search --> : Common mistakes


Sxooter
09-30-2003, 03:10 PM
Here's a couple of common mistakes you see a lot in older php code that can bite you in the butt with 4.3.x.

Unquoted identifiers in an array:

$string['a'] = "text";
print $string[a];

While this would work in older versions of PHP fine, lately, it generates this notice:

Notice: Use of undefined constant abc - assumed 'abc' in - on line x

Also, testing for a value instead of using isset:

if ($varname) {
do_something();
}

If the variable $varname doesn't exist, this will now throw a warning:

Notice: Undefined variable: var in - on line 2

So, if you are writing for PHP 4.3/PHP 5, you should pay attention to these things.

bliss322
09-30-2003, 04:23 PM
since we're on this subject, my recent, big mistake was not having a config file.

when i took the code to another site, i had to change all the instances of database names, tables, session names and such.

session names are the big one for me, that's how i check for admin access and everything.

i had a $_session["admin_user"] on my main site, and then created a subdomain using the same code. because it was a subdomain, that "admin_user" had admin access to my site.

now i have a config file declaring the database info, the table names, and the session names, too.

BuzzLY
09-30-2003, 04:48 PM
As an added security measure, you can put that config file outside your website file structure, so nobody has any access to it at all.

Sxooter
09-30-2003, 05:44 PM
Note that the proper test to see if a var is set, or if it is set AND has a value other than 0, would be something like:

// Is var set?
if (isset($varname)){

}

or

/ Is var set and has a non-zero value:
if (isset($varname)&&$varname){

}

Note that evaluation order means that if the first isset fails, the second $varname check will not be made, and therefore no error. This means that the above check is NOT the same as:

if ($varname&&isset($varname)){

}

since that will result in the first evaluation occuring on a possible non-existent variable.

Good programming practices might seem to demand that you even:


if (isset($varname)){
if ($varname){
}
}


But that's probably overkill.

drawmack
09-30-2003, 08:14 PM
1) Code to a standard, any standard even if it is your own but write it down and have it laying around for future programmers.

I use a variation of: http://utvikler.start.no/code/php_coding_standard.html

2) Document to a standard, and an automated documentation extraction tool is preferable. I use phpdoc - http://www.phpdoc.de/

3) Always, always, always write if statments in this formate if('constant' == $variable) then the parser catches it when you type = instead of ==

4) Use the ! operator in if statments sparingly, pretend it is a contagous disease. It confuses the logic and makes the program harder to read.

5) Never assume register globals is on.

Merve
09-30-2003, 08:43 PM
Use other functions in place of complex array_splice() calls, as in using them in 'if' statements. wordwrap() or other functions will usually do in 2 or 3 lines what array_splice() does in 13 or 14 lines. I know this because I once wrote a wordwrap()-like function using array_splice that took 15 lines...It think. wordwrap() did the same thing in one.

As a general rule, shorten your code! Sift through the manual to find the function you want. If it doesn't exist, make your user-defined function as short as you possibly can.

Also, look through your code and make sure you have closed ALL brackets (brace or curved) this will save you headaches later.

Finally, don't use the delete() function as it doesn't exist. If you think it exists, you're a dumbass.

Moonglobe
10-01-2003, 02:31 AM
Originally posted by Merve
Finally, don't use the delete() function as it doesn't exist. If you think it exists, you're a dumbass.
well then what's this:

delete

:D

yes i know it's just a dummy entry.... but still if it's in the manual....

drawmack
10-01-2003, 03:28 AM
Originally posted by Merve
As a general rule, shorten your code! Sift through the manual to find the function you want. If it doesn't exist, make your user-defined function as short as you possibly can.


in theory this is excellent advice but if you were paying me by the hour I think you'd be a lot happier if I took 10 minutes to code something then if I took an hour to find just the right function.

There are a lot of ''needless'' functions in php. If I can code it in 3 or 4 lines I'm probably not even going to consider that it's already part of the language unless I'm using those 3 or 4 lines all the time.

Jeb.
10-01-2003, 06:05 AM
Don't re-invent the wheel. If there's a class on the Internet that'll do what you need (and there probably is), find it and use it. Of course, pay attention to licensing and what-not.

Modify the class if you have to, but don't write the whole thing from scratch if you can avoid it. This is the biggest problem with programmers - we are architects, and our first instinct is to rip it down and start again. Be careful - this could cost you a lot of development time!

The only time I rewrite something from scratch is if the license it's under prevents me from doing what I want with it. EG, I had to rewrite portions of the Eclipse library because of the license it was under (LGPL) - I couldn't allow the source of it to be viewable to the general public.

While I'm on the subject of classes, get a class library together. Have a whole bunch of classes doing different (but unique) things. That way when you need them, you just pull up your library, grab the class, whack it in and there you go.

Merve
10-01-2003, 09:53 PM
Excellent point drawmack. I didn't think it up.

Also, if you have an if, and an else, but no elseif, consider using the ternary operator...shortens your code a lot...and if you have a huge if-elseif-else structure with only one variable in the condition, consider using the switch statement. It is much better for debugging purposes...well the ternary operator is not always better for debugging purposes...but you get the picture. Short code is good. Long code with lots of loops is bad. Only use loops if you absolutely have to and it is the shortest way to execute the code.

drawmack
10-02-2003, 10:00 AM
Tom like loops

I am the loop god
the loop to the dog
or the pool to the god
;)

Weedpacket
10-03-2003, 02:15 AM
Originally posted by drawmack
in theory this is excellent advice but if you were paying me by the hour I think you'd be a lot happier if I took 10 minutes to code something then if I took an hour to find just the right function.

There are a lot of ''needless'' functions in php. If I can code it in 3 or 4 lines I'm probably not even going to consider that it's already part of the language unless I'm using those 3 or 4 lines all the time. You know, that sounds not too dissimilar to my some-time code early, code often (http://www.phpbuilder.com/board/showthread.php?s=&postid=10278869#post10278869) approach.

[PS; As for taking ten minutes instead of an hour, my rates tend to be in increments of 30 minutes. So taking ten minutes saves me fifty but the customer only thirty; I might as well spend those other twenty minutes on something useful.... I need a coffee.]

drawmack
10-03-2003, 07:55 AM
Weed, that's exactly what I was getting at and your article reminded me of another point.

place every sql statement for the web-site into a named array and have the function use them from here. This way a database change is easy to propogate through the site.

Actually what I've done is better then that. Write a generic database class that uses the information the database will give you and a table in the database to do all database stuff then you just make the change to the database and change the control table and you've updated the access methods as well.

BuzzLY
10-03-2003, 10:43 AM
What a great idea. What would you call that? I think you should call it "PEAR." :D

drawmack
10-03-2003, 06:18 PM
Buzz, nah mine draws the input screens and stuff too. I like mine better then PEAR but a lot of people might disagree.

Moonglobe
10-03-2003, 06:19 PM
Post it then. :)