I found myself giving out similar advice a lot last week to beginners trying to make a contact form.

I decided to try to make a contact form that is easy enough for beginners to understand (if they bother to read the code and look at the git commits) while also trying to nudge them in the right direction so that someday soon they'll try writing better organized code.

1. I might use URLs with PATH_INFO instead of a 'cmd' parameter. It would be clearer what was the route and what were the form parameters.

2. I would name the HTML functions using a 'html' prefix, so htmlList() rather than arrayToList() and htmlSelect() rather than buildSelect(). That would make it clearer what is an HTML generator and what is an array function.

3. Why are the HTML helpers functions, but the array helper a class? It might be better to be consistent.

4. I might split the Controllers out into separate PHP files, even if showSuccess ends up as a one line controller. That would show the difference between the Router/Front Controller and the Page Controllers. Or if in one file maybe have the URLs be 'emailform/validate/' to show multiple Actions within a Controller.

1. $_SERVER['PATH_INFO']? I've never used that before. I'll give it a shot.

2. Will do.

3. Why are the HTML helpers functions, but the array helper a class? It might be better to be consistent. The array helper would have to use $_POST or another global variable to work the way I wanted it to work. Maybe I should rename the folder to "helpers"? Or should I keep classes separate from functions? Or should I put the html helpers into an HtmlHelpers class and then use Composers no-namespace autoloading feature to load classes from the helpers folder?

4. I'm not certain what you're sayin for #4. I wanted to stick with "contact.php" because most beginners I've encountered don't seem enthusiastic about defining routes in a front controller for some reason.

5. Will do.

6. I've honestly never used filter_var before this I'll give it a shot.

3. I think being consistent would be good. I think if you can make it minimally OO that would be good too. Maybe put them all in a "contact/" folder so they go with the script if you want to keep everything easy to install.

4. I think it is fine to keep it all in one contact.php script. I would make clear what is Controller code and what is View code.

Have you thought about how they will style the page? Maybe you should put the form code output into the middle of a wrapper template that they can customize to look like their site?

Gave this a quick look this morning but didn't have time to review it until now. I'm trying to keep in mind that this is aimed at beginners, and have had a bit of a tough time not being able to review things inline, but here are a few thoughts:

1. Clean up the formatting. It's a mess. PSR-2 is probably best, but whatever you use, just be consistent.

2. Comment your code. Functions/methods should have a doc block explaining what they do, what parameters they take -- type, name, and description -- and a return type. This is perhaps doubly important if this is aimed at beginners.

3. route_to_appropriate_function just barfs out an exit. Might be a good place to introduce both exceptions and proper use of HTTP response codes.

4. validate should probably be split into two functions; one that performs validation and another that builds up the output.

5. ArrayCompare doesn't actually compare arrays. Naming matters.

5.a. Use meaningful method names. ArrayCompare::compare is only three more characters but much clearer.

6. phpmailer-gmail. Is there much value in this? Wouldn't a more generic helper be more useful? Gmail-specific properties can be passed in through config.

That's what my contact.php is an example of. Lines 1, 2, 3, and 24 is what they would add to their own php page file.

My point was if they already have a layout template, then they would have to duplicate that into your file -- which means double maintenance down the road. Better if your code was designed to put into an external template file. Then they could use their or modify yours. But, I understand this is for beginners.

2. Comment your code. Functions/methods should have a doc block explaining what they do, what parameters they take -- type, name, and description -- and a return type. This is perhaps doubly important if this is aimed at beginners.

I would always vote against using comments for function/method/params description. The function/method/param name should be sufficient (and the only place) to describe what it does. The need of adding any description comment is a code smell IMHO. Another thing is that sooner or later the comment becomes a lie if not maintained together with code.

Still, adding doc blocks for parameters/return type is OK especially for some IDEs that utilize it (e.g. Netbeans) and for such IDEs most of it is autogenerated, so it doesn't become a "lie" as code evolves.

_________________There are 10 types of people in this world, those who understand binary and those who don't

Who is online

You cannot post new topics in this forumYou cannot reply to topics in this forumYou cannot edit your posts in this forumYou cannot delete your posts in this forumYou cannot post attachments in this forum