PHP Developers Network

A community of PHP developers offering assistance, advice, discussion, and friendship.
 
Loading
It is currently Wed Sep 19, 2018 10:10 pm

All times are UTC - 5 hours




Post new topic Reply to topic  [ 26 posts ]  Go to page 1, 2  Next
Author Message
PostPosted: Sun Feb 11, 2018 9:07 pm 
Offline
Forum Contributor

Joined: Fri Jul 20, 2007 11:29 am
Posts: 366
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: 13576
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: 366
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: 13576
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: 366
#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: 6424
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: 366
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: 13576
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: 4313
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  
 
PostPosted: Mon Mar 12, 2018 10:47 am 
Offline
Forum Contributor

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


An infrequently used function with several different outputs based on the input params definitely benefits from a brief reminder of how that function works and a lengthy description of how it's being used in that location. If you're going to say "well that function is obviously smelly" don't blame me, blame the php devs.

VladSun wrote:
Another thing is that sooner or later the comment becomes a lie if not maintained together with code.

This is only true of a few functions. I have a few dozen projects between 6 and 12 years old where I had to maintain a few comments, but over 99% of the comments never needed to be touched, even on functions that have been totally rewritten for clarity/security/performance.

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


Top
 Profile  
 
PostPosted: Mon Mar 12, 2018 11:24 am 
Offline
Forum Contributor

Joined: Fri Jul 20, 2007 11:29 am
Posts: 366
Christopher wrote:
1. I might use URLs with PATH_INFO instead of a 'cmd' parameter.


Could you give me an example?

I vaguely remember back in the day some sites would have urls like "example.com/index.php/services"

I'm trying that on my server and it loads index.php but without the css. It seems my browser treats the "/foo" after the .php file name as a directory which means I'll have to make any css and js files absolute urls or maybe adjust something in my apache config.

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


Top
 Profile  
 
PostPosted: Mon Mar 12, 2018 10:04 pm 
Offline
Site Administrator
User avatar

Joined: Wed Aug 25, 2004 7:54 pm
Posts: 13576
Location: New York, NY, US
You can do it that way, but you may need to adjust your scripts and style paths. And use rewriting to allow even cleaner URLs. There are lots of example of the rewrite config settings.

_________________
(#10850)


Top
 Profile  
 
PostPosted: Mon Mar 12, 2018 11:22 pm 
Offline
Forum Contributor

Joined: Fri Jul 20, 2007 11:29 am
Posts: 366
Christopher wrote:
You can do it that way


So it sounds like what you were suggesting is something I'm not familar with. I've never used http://php.net/pathinfo before. Could you give me an example of how you meant I should be using it?

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


Top
 Profile  
 
PostPosted: Wed Mar 14, 2018 1:56 pm 
Offline
Site Administrator
User avatar

Joined: Wed Aug 25, 2004 7:54 pm
Posts: 13576
Location: New York, NY, US
Well, there are a couple things relating to user "clean URLs" that you have run into. For example, I use a common outer template for most of my sites that has the <base> tag set in the header. You can use rewrite rules in the web server or not to eliminate the need for 'index.php/' in the URL.

The information for the route is available in $_SERVER['PATH_INFO'] from the request, not the pathinfo() function that parses paths.

_________________
(#10850)


Top
 Profile  
 
PostPosted: Wed Mar 14, 2018 8:57 pm 
Offline
Forum Contributor

Joined: Fri Jul 20, 2007 11:29 am
Posts: 366
Ok I'll skip that feature for this project. The people I had in mind when I started this are still at basic "I need a form for my contact.php" level.

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


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 26 posts ]  Go to page 1, 2  Next

All times are UTC - 5 hours


Who is online

Users browsing this forum: Majestic-12 [Bot] and 5 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