User registration

[agoe]

Hi, as the subject states, i've written a user registration-script.

Please comment on any flaws, efficency issues or other things.

you type your nickname and email-adresse, and then a generated password is sent to you. hope the code is readable.

all the html except from the form elements is removed, so don't bother thinking about how it all looks.

also, say if you like or dislike the code

code:
http://anders.ownage.no/code/register.phps

[Reformed]

The first thing I noticed is that you don't have one comment in the entire script. Very important to comment. Otherwise it looks good.

[BuzzLY]

Is there some reason why your server doesn't color-code the source code? most phps files are color-coded, and therefore easier to read.

[planetsim]

A couple of issues.

register_final($username, $email)

The arguments are from Posted Forms. Now $_POST is a superglobal so having those two arguments are just a waste.

PHP Code:

echo "name: <input type=\"Text\" maxlength=\"20\" name=\"dusername\">";


Can be this

PHP Code:

echo 'name: <input type="Text" maxlength="20" name="dusername">';


Just looks neater to me

PHP Code:

function logoff() {
    $_SESSION=array();
    session_destroy();
}


Could be done easier

PHP Code:

function logoff() {
    unset$_SESSION['session_name'])
}


session_destroy is better used when you use session_register(); and i dont see it in the script and if you are you shouldnt be using $_SESSION.

Also SQL Injections. It doesnt look like you are trying to prevent those at all, which is a huge security risk.

Best link i know for you on how to prevent for them http://www.sitepoint.com/article/794

[Weedpacket]

Originally posted by planetsim

PHP Code:

echo "name: <input type=\"Text\" maxlength=\"20\" name=\"dusername\">";


Can be this

PHP Code:

echo 'name: <input type="Text" maxlength="20" name="dusername">';


Just looks neater to me

You could also go further than that, noting that strings can run across more than one line, and there's also the heredoc syntax:

PHP Code:

echo 'name: <input type="Text" maxlength="20" name="dusername">
e-mail: <input type="text" maxlength="50" name="email">
confirm e-mail: <input maxlength="50" type="text" name="cemail">
<input type="submit" name="dsubm" value="next">';

echo <<< EOF
name: <input type="Text" maxlength="20" name="dusername">
e-mail: <input type="text" maxlength="50" name="email">
confirm e-mail: <input maxlength="50" type="text" name="cemail">
<input type="submit" name="dsubm" value="next">
EOF;


or even go the whole hog and escape out of PHP completely for the duration.

[lastcraft]

Hi...

The first thing I noticed is that you don't have one comment in the entire script. Very important to comment. Otherwise it looks good.

This comment thing gets out of hand I feel. the main objective is clear maintainable code. Comments that just repeat the function names just add clutter. Here we have well chosen names, making the more obvious comments superfluous.

The really useful comments are things that humans would say. Such things as "To do..." or "This bit really sucks..." or "This can be rather slow when...".

PHPDoc comments are different. They are there so that you don't even have to look at the code. Some repetition is inevitable in this case, but as the point of this forum is code, there is little point in putting PHPDoc blocks in.

yours, Marcus

[lastcraft]

Hi...

The arguments are from Posted Forms. Now $_POST is a superglobal so having those two arguments are just a waste.

Ouch.

I would go the other way and actually pass all the POST data in from the top level function. Makes everything much easier to test and you can then reuse the code in other places where you happen to use GET. By using any global data anywhere in a function, you are reducing flexibility.

Super purist me .

yours, Marcus