meOmy Posted September 17, 2023 Share Posted September 17, 2023 Hello, I'm just playing around with PHP (I love it but am not very good at it) and now trying to find ways to clean things up and/or reduce the amount of code I see on any given page. My problem is: I have a lot more validation (preg_match) than I'm showing below, but to make it easier for you, I've removed all but two of them. When I look at that page, I think to myself, why not put all the validation in a separate file and then INCLUDE/REQUIRE the file that has the validation inside it? However, that doesn't work. I even thought maybe there's a way to do a function with all the validation in it and place that inside the IF/ELSE. However, I'm not very good at/with functions, so that solution has eluded me. The fact I can't seem to find an answer on the Internet is leading me to believe I'm going about it the wrong way. How do I remove the validation part from where it is right now and put it in a separate file or function and use/call it, so the page works? Thank you in advance for any help you can provide. if ($_SERVER['REQUEST_METHOD'] === 'POST') { if (!preg_match("/^[a-zA-Z\'\-\s]*$/", $firstname) == TRUE) { $firstnameErr2 = "Only letters allowed"; } if (!preg_match("/^[a-zA-Z\'\-\s]*$/", $lastname) == TRUE) { $lastnameErr = "Only letters allowed"; } else { $firstname = sanitize_input($_POST["firstname"]); $lastname = sanitize_input($_POST["lastname"]); $address1 = sanitize_input($_POST["address1"]); $address2 = sanitize_input($_POST["address2"]); $city = sanitize_input($_POST["city"]); $state = sanitize_input($_POST["state"]); $zipcode = sanitize_input($_POST["zipcode"]); $phone = localize_us_number($_POST["phone"]); $stmt = $pdo->prepare("SELECT username FROM users WHERE username=?"); $stmt->execute([$myusername]); $user = $stmt->fetch(); if ($user) { if ($_SESSION["usertype"] == 5) { $sql = "UPDATE users SET active_state=? WHERE username = ?"; $stmt = $pdo->prepare($sql)->execute([$active_state, $myusername]); } $sql = "UPDATE contact SET firstname=?, lastname=?, address1=?, address2=?, city=?, state=?, zipcode=?, phone=? WHERE username = ?"; $stmt = $pdo->prepare($sql)->execute([$firstname, $lastname, $address1, $address2, $city, $state, $zipcode, $phone, $myusername]); } else { $stmt = $pdo->prepare("INSERT INTO contact (firstname, lastname, address1, address2, city, state, zipcode, phone) VALUES (?, ?, ?, ?, ?, ?, ?, ?) "); $stmt->execute([$firstname, $lastname, $address1, $address2, $city, $state, $zipcode, $phone]); } } } Quote Link to comment https://forums.phpfreaks.com/topic/317293-php-require-or-include-inside-of-ifelse-statement/ Share on other sites More sharing options...
meOmy Posted September 17, 2023 Author Share Posted September 17, 2023 OOP is beyond me, so I'm doing procedural. Quote Link to comment https://forums.phpfreaks.com/topic/317293-php-require-or-include-inside-of-ifelse-statement/#findComment-1611916 Share on other sites More sharing options...
requinix Posted September 18, 2023 Share Posted September 18, 2023 8 hours ago, meOmy said: When I look at that page, I think to myself, why not put all the validation in a separate file and then INCLUDE/REQUIRE the file that has the validation inside it? However, that doesn't work. I even thought maybe there's a way to do a function with all the validation in it and place that inside the IF/ELSE. Or do both: put the validation inside a function, and put the function inside a file. <?php function is_valid_name($name) { return preg_match("/^[a-zA-Z\'\-\s]*$/", $name); } if ($_SERVER['REQUEST_METHOD'] === 'POST') { require_once "the other file.php"; if (!is_valid_name($firstname)) { $firstnameErr2 = "Only letters allowed"; } if (!is_valid_name($lastname)) { $lastnameErr = "Only letters allowed"; } 8 hours ago, meOmy said: The fact I can't seem to find an answer on the Internet is leading me to believe I'm going about it the wrong way. Kinda. I'd say that you're going about it the naive and simplistic way. Which is typically the first step towards doing it in a smarter and more flexible way. If you're doing this for a personal project then don't mind it. Unless you specifically want to learn how to do this better...? But I will say one thing: don't validate people's names. Just don't. It's a bad practice because you're always going to end up rejecting the valid name of someone, somewhere. Additionally, separating names by first and last isn't even correct either: there are people with single names. The right way to do names is to use a single full-name input and ensure it has something in it. Nothing more. 1 Quote Link to comment https://forums.phpfreaks.com/topic/317293-php-require-or-include-inside-of-ifelse-statement/#findComment-1611921 Share on other sites More sharing options...
meOmy Posted September 18, 2023 Author Share Posted September 18, 2023 Thanks for the name tip. Yes, it's a personal learning project, but I want to learn to do it better. Quote Link to comment https://forums.phpfreaks.com/topic/317293-php-require-or-include-inside-of-ifelse-statement/#findComment-1611922 Share on other sites More sharing options...
meOmy Posted September 18, 2023 Author Share Posted September 18, 2023 Goodness! I must have been looking at this too quickly because I didn't even realize you actually showed me a function and a solution. Let me process this. Thank you for your time! Quote Link to comment https://forums.phpfreaks.com/topic/317293-php-require-or-include-inside-of-ifelse-statement/#findComment-1611923 Share on other sites More sharing options...
meOmy Posted September 18, 2023 Author Share Posted September 18, 2023 So, this is what I have now, but NOW the if statements for validation are not halting the process. It's showing the error, but it continues on to the ELSE portion. Why is it no longer stopping when I purposely put a number inside an input that only allows/calls for letters? I'm not getting any PHP display errors, but I don't really know how to test an IF statement to see what's happening in the background. This used to work, but somewhere down the line with all my changes it stopped working and i can't find anything on the Internet that addresses my particular case. Thank you. if ($_SERVER['REQUEST_METHOD'] === 'POST') { require_once("../_includes/functions/validateInput.php"); if (!is_valid_firstname($firstname)) { $firstnameErr = "Only letters allowed"; } if (!is_valid_lastname($lastname)) { $lastnameErr = "Only letters allowed"; } if (!is_valid_address1($address1)) { $address1Err = "Only letters & numbers allowed"; } if (!is_valid_address2($address2)) { $address2Err = "Only letters & numbers allowed"; } if (!is_valid_city($city)) { $cityErr = "Only letters allowed"; } if (!is_valid_zipcode($zipcode)) { $zipcodeErr = "Only numbers allowed"; } if (!is_valid_phone($phone)) { $phoneErr = "Only numbers allowed"; } else { $firstname = sanitize_input($_POST["firstname"]); $lastname = sanitize_input($_POST["lastname"]); $address1 = sanitize_input($_POST["address1"]); $address2 = sanitize_input($_POST["address2"]); $city = sanitize_input($_POST["city"]); $state = sanitize_input($_POST["state"]); $zipcode = sanitize_input($_POST["zipcode"]); $phone = localize_us_number($_POST["phone"]); $stmt = $pdo->prepare("SELECT username FROM users WHERE username=?"); $stmt->execute([$myusername]); $user = $stmt->fetch(); if ($user) { if ($_SESSION["usertype"] == 5) { // The file templates/contact.php is used by both user and admin. However, when used by admin, it's necessary to update active_state. // This section not required by user as they can't update their active_state. $sql = "UPDATE users SET active_state=? WHERE username = ?"; $stmt = $pdo->prepare($sql)->execute([$active_state, $myusername]); } $sql = "UPDATE contact SET firstname=?, lastname=?, address1=?, address2=?, city=?, state=?, zipcode=?, phone=? WHERE username = ?"; $stmt = $pdo->prepare($sql)->execute([$firstname, $lastname, $address1, $address2, $city, $state, $zipcode, $phone, $myusername]); } else { $stmt = $pdo->prepare("INSERT INTO contact (firstname, lastname, address1, address2, city, state, zipcode, phone) VALUES (?, ?, ?, ?, ?, ?, ?, ?) "); $stmt->execute([$firstname, $lastname, $address1, $address2, $city, $state, $zipcode, $phone]); } } } Quote Link to comment https://forums.phpfreaks.com/topic/317293-php-require-or-include-inside-of-ifelse-statement/#findComment-1611947 Share on other sites More sharing options...
meOmy Posted September 18, 2023 Author Share Posted September 18, 2023 Oh, and you may want this, too. It's the validateInput.php file being called: // check for letters, apostrophe (\'), hypens (\-), and allow empty (\s) function is_valid_firstname($firstname) { return preg_match("/^[a-zA-Z\'\-\s]*$/", $firstname); } // check for letters, hyphens (\-), and allow empty (\s) function is_valid_lastname($lastname) { return preg_match("/^[a-zA-Z\'\-\s]*$/", $lastname); } // check for letters, numbers, hyphens (\-), and allow empty (\s) function is_valid_address1($address1) { return preg_match("/^[a-zA-Z0-9\-\s]*$/", $address1); } // check for letters, numbers, hyphens (\-), and allow empty (\s) function is_valid_address2($address2) { return preg_match("/^[a-zA-Z0-9\-\s]*$/", $address2); } // check for letters and allow empty (\s) function is_valid_city($city) { return preg_match("/^[a-zA-Z\s]*$/", $city); } // check for numbers and allow empty (\s) function is_valid_zipcode($zipcode) { return preg_match("/^[0-9\s]*$/", $zipcode); } // check for numbers and allow empty (\s) function is_valid_phone($phone) { return preg_match("/^[0-9\-\s]*$/", $phone); } Quote Link to comment https://forums.phpfreaks.com/topic/317293-php-require-or-include-inside-of-ifelse-statement/#findComment-1611948 Share on other sites More sharing options...
Solution mac_gyver Posted September 19, 2023 Solution Share Posted September 19, 2023 (edited) 56 minutes ago, meOmy said: but NOW the if statements for validation are not halting the process because you have no logic to do this. the else() you have is part of the last if(){} conditional, for the phone number. you should NOT use discrete variables for the error messages. this would require you to either have an additional flag to indicate any errors or you would need to write a conditional test with all the discrete variables. you should instead use an array (arrays are for sets of things where you are going to operate on each member in the set in the same or similar way) to hold the user/validation errors, using the field name as the main array index. after the end of the validation logic, if the array holding the errors is empty, you can use the submitted form data, e.g. - if(empty($errors)) { // use the submitted data here... } to display the errors at the appropriate location in the html document, either test the contents of the array and display all the errors together or display each error adjacent to the field it corresponds to. speaking of/writing about using arrays for sets of things, you should keep the form data as a set in a php array variable, then operate on elements in this array variable throughput the rest of the code. this will allow you to trim all the data at once, using one php array function, and will support dynamically validating and processing the data. speaking of/writing about dynamically validating the data, you should NOT write out - copy/paste logic for every possible value. you should instead use a data-driven design, where you have a data structure (database table, array) that holds the dynamic values, then use this definition to control what general purpose logic does. the only thing that is different between these validation types is the regex pattern. why not store them in an array with the data type, regex pattern and error message, then just get the correct entry and call one general purpose function with the correct type regex pattern and the input value? 56 minutes ago, meOmy said: sanitize_input() whatever the code is for this function is probably improper. in general, besides trimming input data, you should NOT modify user submitted data as this changes the meaning of the data. if data is valid, use it. if it is not, tell the user what is wrong with it and let the user fix then resubmit the data. Edited September 19, 2023 by mac_gyver Quote Link to comment https://forums.phpfreaks.com/topic/317293-php-require-or-include-inside-of-ifelse-statement/#findComment-1611949 Share on other sites More sharing options...
meOmy Posted September 19, 2023 Author Share Posted September 19, 2023 Quote because you have no logic to do this. the else() you have is part of the last if(){} conditional, for the phone number. Thank you for that. It never occurred to me, but I've since tested it and finally see exactly what you mean. Quote whatever the code is for this function is probably improper. in general, besides trimming input data, you should NOT modify user submitted data Just to follow up with your comment, so you can see what it was. function sanitize_input($data) { $data = trim($data); $data = stripslashes($data); $data = htmlspecialchars($data); return $data; } I don't have much learning experience with arrays as you suggest to use for errors, but when I get back from FL next week, I will be definitely going down the 'rabbit hole' on: if(empty($errors)) { // use the submitted data here... } Thank you very much for taking the time to write such a lengthy response. It was/is greatly appreciated. Quote Link to comment https://forums.phpfreaks.com/topic/317293-php-require-or-include-inside-of-ifelse-statement/#findComment-1611957 Share on other sites More sharing options...
mac_gyver Posted September 19, 2023 Share Posted September 19, 2023 2 hours ago, meOmy said: so you can see what it was. this is nonsense code that has been floating around on the web for a long time. the only thing it is doing that is proper is trimming the value. stripslashes, when it was needed, should have been conditionally applied only when magic_quotes_gpc was on, but the (bad) feature in php that needed stripslashes to be applied to input data was removed from php in 2012, over a decade ago. htmlspecialchars is an output function. it is applied to data being used in a html context. it is not used on input data that your script will use for database operations as it modifies the value. so, even if this function only trimmed the data, the proper place to use it would have been before validating the data, so that you would be validating the trimmed values. Quote Link to comment https://forums.phpfreaks.com/topic/317293-php-require-or-include-inside-of-ifelse-statement/#findComment-1611962 Share on other sites More sharing options...
meOmy Posted September 29, 2023 Author Share Posted September 29, 2023 Thank you both very much for your help. Based on your replies, I was able to create/use an array for the very first time. I also thought that sanitizeInput function looked wrong and outdated. I'm glad you took the time to mention it. It confirmed my suspicion and I removed all but the trim portion. PHP is a lot of fun to learn and I'm grateful to both of you for your assistance. Sometimes, I just can't find a solution no matter how much I search, and your help is greatly appreciated to keep going. Quote Link to comment https://forums.phpfreaks.com/topic/317293-php-require-or-include-inside-of-ifelse-statement/#findComment-1612190 Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.