PHP Developers Network

A community of PHP developers offering assistance, advice, discussion, and friendship.
 
Loading
It is currently Thu Feb 22, 2018 1:44 am

All times are UTC - 5 hours




Post new topic Reply to topic  [ 9 posts ] 
Author Message
PostPosted: Sun Feb 11, 2018 9:07 pm 
Offline
Forum Contributor

Joined: Fri Jul 20, 2007 11:29 am
Posts: 350
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.

Please have a look and give suggestions if you have the time.

Remember it's meant to try to be beginner friendly.

https://github.com/thinsoldier/php-beginner-form

_________________
Warning: I have no idea what I'm talking about.


Top
 Profile  
 
PostPosted: Mon Feb 12, 2018 11:58 am 
Offline
Site Administrator
User avatar

Joined: Wed Aug 25, 2004 7:54 pm
Posts: 13500
Location: New York, NY, US
I think it looks good. I have a few comments.

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.

5. I think ( ! isset( $input[$field]) || empty($input[$field] ) can be just ( ! empty($input[$field] )

6. I would use filter_var() on every field.

_________________
(#10850)


Top
 Profile  
 
PostPosted: Mon Feb 12, 2018 3:00 pm 
Offline
Forum Contributor

Joined: Fri Jul 20, 2007 11:29 am
Posts: 350
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 :banghead: I'll give it a shot.

_________________
Warning: I have no idea what I'm talking about.


Top
 Profile  
 
PostPosted: Mon Feb 12, 2018 3:19 pm 
Offline
Site Administrator
User avatar

Joined: Wed Aug 25, 2004 7:54 pm
Posts: 13500
Location: New York, NY, US
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?

_________________
(#10850)


Top
 Profile  
 
PostPosted: Mon Feb 12, 2018 6:28 pm 
Offline
Forum Contributor

Joined: Fri Jul 20, 2007 11:29 am
Posts: 350
#4 https://github.com/thinsoldier/php-begi ... ontact.php

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.

_________________
Warning: I have no idea what I'm talking about.


Last edited by thinsoldier on Tue Feb 13, 2018 12:55 am, edited 1 time in total.

Top
 Profile  
 
PostPosted: Mon Feb 12, 2018 8:22 pm 
Offline
Moderator
User avatar

Joined: Tue Nov 09, 2010 3:39 pm
Posts: 6420
Location: Montreal, Canada
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.

_________________
Supported PHP versions No longer supported versions


Top
 Profile  
 
PostPosted: Thu Feb 15, 2018 2:10 pm 
Offline
Forum Contributor

Joined: Fri Jul 20, 2007 11:29 am
Posts: 350
My comment didn't save. Short version: Yes, Celauran... most of that, will do.

_________________
Warning: I have no idea what I'm talking about.


Top
 Profile  
 
PostPosted: Thu Feb 15, 2018 8:16 pm 
Offline
Site Administrator
User avatar

Joined: Wed Aug 25, 2004 7:54 pm
Posts: 13500
Location: New York, NY, US
thinsoldier wrote:
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.

_________________
(#10850)


Top
 Profile  
 
PostPosted: Sat Feb 17, 2018 8:42 am 
Offline
DevNet Master
User avatar

Joined: Wed Jun 27, 2007 9:44 am
Posts: 4311
Location: Sofia, Bulgaria
Celauran wrote:
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


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 9 posts ] 

All times are UTC - 5 hours


Who is online

Users browsing this forum: No registered users and 17 guests


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

Jump to:  
Powered by phpBB® Forum Software © phpBB Group