benanamen

Aside from setting a default value, is there any reason a class needs properties or a property with no default value? Here are two examples I am working with. One has a default car color from a property that gets changed (overwritten?), the other just sets the color with no property. Property with default color value <?php class Car { public $color = 'red'; } $bmw = new Car (); echo $bmw -> color; // red $bmw -> color = 'blue'; echo "<br>"; echo $bmw -> color; // blue Class with no property sets color <?php class Car { } $bmw = new Car (); $bmw -> color = 'blue'; echo $bmw -> color; // blue

You are using obsolete code that has been removed from Php. You need to use PDO with prepared statements. You are also vulnerable to an SQL Injection Attack. You never ever use variables in your query. The entire thing needs to be tossed. https://phpdelusions.net/pdo

Look at your first if. Your POST is wrong. $POST should be $_POST

If you don't see it, it is because I don't know to do it, otherwise it would be there. So perhaps that is actually the next part I need to learn about. I don't know about attributes holding state in OOP. I don't know about autonomously taking care of tasks in OOP. What can you tell me about that in a way I can understand? At this point, I don't know enough to make decisions of how I would personally want to implement anything in OOP. I am looking for the definitive way to do things for as much as that is possible. One of the things I don't like about Php is there is just way too many ways to do the same exact thing. One of the things I liked about implementing interfaces is it is definitive in that if you don't use the methods, it doesn't work. It is a strict rule.

I think you are not understanding my approach. I know there is much more to be done. I am basically taking this a piece at a time to make sure I am getting that piece correct and trying to avoid information overload. What you see "left over" in procedural is what are the next steps to understand in OOP correctly. For a DB, making a connection is step one. Now obviously I need to learn the proper way to handle CRUD. When I ask you questions, it is not to question what you tell me, it is to understand the "why" and not just to know that's what I should do. e.g I know to turn on a light you flip the switch and the light comes on, but I want to know "why" it does. You can't really be a Master of something if you don't know it inside and out. That is not my intent despite what the readme on my repo says. There is a particular reason it was worded like that. My intent is to re-write that project in complete, optimum OOP best practices. I have read numerous car, bicycle, animal, and people OOP tutorials and am familiar with what they all said. How much is correct or could be better, I have no idea as of now. So, you know my project, you know what my intent is, and you can see the code at my repo that I am transforming to OOP. If you're willing, that should make it easy to guide me, I am more than willing to put in the time to learn and understand what you tell me. So, yes, I do want to learn OOP, but I want to learn it correctly from the start. Moving forward, it seems my next steps are to do something with the CRUD operations. And yes, I have read many a code that others have done which just appears they are probably doing it wrong since none of them do it near the same way. My question is, would the CRUD operation for a particular DB type (e.g Mysql) go in the class that implements the connection or is there some super crud class design that any type of db can use? Additionally, since you could be using two DB's at once (Mysql & MongDB), how should the actual credentials be set up? Thanks to all who help!

Your code is vulnerable to an SQL Injection Attack. You never ever put variables in a query. You need to use prepared statements. It is also a security problem outputting DB errors to the user. That info is only good to a programmer or a hacker. I suspect you have many other problems as well.

Where is the array coming from? A Database? How is the data getting like that in the first place?

Looking into it, that looks like what I needed to know about. I see Active Record is a pattern as well as an Open Source ORM library. Which one are you referring to? Is the pattern something I should learn how to write myself or do I just use the library? This works exactly as expected. Any issues with it? Since this works, why would I need a Data Mapper or Active Record mapper? <?php interface Database { public function connect(); } //---------------------------------------------------------------------------------------- // //---------------------------------------------------------------------------------------- class Mysql implements Database { public function connect() { $dsn = DB_TYPE . ":host=" . DB_HOST . ";dbname=" . DB_NAME . ";charset=" . DB_CHARSET; $opt = [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, PDO::ATTR_EMULATE_PREPARES => false, ]; return new PDO($dsn, DB_USER, DB_PASSWORD, $opt); } } //---------------------------------------------------------------------------------------- // Testing //---------------------------------------------------------------------------------------- $db = new Mysql(); $pdo = $db->connect(); $sql = 'SELECT login_username FROM user_login'; $stmt = $pdo->prepare($sql); foreach ($pdo->query($sql) as $row) { echo $row['login_username']; }

The short answer to what I am doing is converting the procedural code from my repo to OOP for the sole purpose of learning advanced OOP. For as much as is possible, I want to write the "perfect app" in OOP.

As mentioned in another thread, all the OOP questions are for learning the proper way to do OOP. The test project only consists of registration/login/forgot password/password reset functionality so that would tell you what all my questions will relate to. When I post something that doesn't make sense in relation to that functionality it is because that is the best I currently understand to do in OOP so knowing what the "project" consists of should make it easy for someone to guide me and point me in the proper direction. I could read about how to do OOP all day on the net but I don't know enough yet to know if the author knows what they are talking about so I look to the couple experts on this forum that I trust and know have the proper knowledge. When I see certain procedural code I can spot in an instant if the author knows anything. i.e vars in a query, mysql_* functions. In OOP I currently can only spot if they use var or it looks like the class does a lot of things. So to answer your question (which knowing the project should make obvious) 1. I want a person to be able to register for the app 2. Validate their email address from a secure token 3. Be able to login and logout 4. Submit a "forgot password" request 5. Reset the password from a token emailed So this deals with a database, authentication, sessions etc, the usual stuff you already know. I have done this hundreds of times in procedural. For the OOP version, it should be easily scalable to other implementations such as different DB's, different logging mechanisms, etc, etc. What you taught me about interfaces was a HUGE bump in the right direction. I would have been flopping around with abstract classes or something less important if it wasn't for you.

That's your "Expert" advice to someone trying to learn? Why even bother responding with such a worthless post?

Are my questions getting too advanced for this forum now?

Command line of what? Are you on Windows or Linux?

Based on what I have learned from this thread this is how I am thinking a database interface should be done. Do I have this right?

You don't seem to realize what your real problem is. This is a classic XY Problem. See my signature for an explanation. You either want to do it right or you don't. Your code, including what you didnt post is junk and not fixable. It requires a complete rewrite. If you just want it to "work" until you're on a server with current Php and it just plain won't work no matter what, then someone else can "help" you. What happened to "open to new ways"?

Okay, for starters, study this PDO tutorial and let me know when you have a grasp of it and we will go from there. The very first thing you're going to do is use PDO instead of the obsolete mysql_* code. https://phpdelusions.net/pdo You are also going to stop using REQUEST and your going to stop echoing HTML. It would be a good idea to post an sql dump of your DB if you are able. You can also PM it to me. 1. I need to make sure it is correct 2. I can follow along with something I can run * While not a problem, an id column is named id or id_somthingrelated. iid is a bit odd. (Yeah, i get that it probably stands for inventory id.)

What nobody has mentioned is that your code is obsolete and insecure and has been completely removed from Php. If you want to do this correctly and have the freedom to change code as required we can talk. If you are of the mindset that you just want what you have to "work", or "have to" use this code then you are on your own as far as I go. The code is just all kinds of wrong.

That is already in the code posted. class PHPMailSubmissionAgent implements MailSubmissionAgent

Write a coherent question and you might get some help.

I have integrated and tested the mailing Interface and all works as expect. If there is no further changes needed I would like to move on to the following that I am not for sure where to begin. I assume it is something similar to the other Interfaces. I am guessing this is what you mean. Am I on the right track? <?php interface Database { public function Connection(); } class ConnectMysql implements Database { public function Connection() { $dsn = DB_TYPE . ":host=" . DB_HOST . ";dbname=" . DB_NAME . ";charset=" . DB_CHARSET; $opt = [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, PDO::ATTR_EMULATE_PREPARES => false, ]; try { $pdo = new PDO($dsn, DB_USER, DB_PASSWORD, $opt); } catch(PDOException $e) { $error = $e->getMessage() . ' in ' . $e->getFile() . ' on line ' . $e->getLine(); error_log(MYSQL_DATETIME_TODAY . "|$error\r\n", 3, ERROR_LOG_PATH); } } RegisterUser.php (Current Working) <?php /** * Interface MailSubmissionAgent */ interface MailSubmissionAgent { /** * Sends an e-mail to a single address * * @param $from string the sender address * @param $to string the receiver address * @param $subject string the mail subject * @param $body string the mail body */ public function send($from, $to, $subject, $body); } // ---------------------------------------------------------------------------------------- // // ---------------------------------------------------------------------------------------- class PHPMailSubmissionAgent implements MailSubmissionAgent { /** * @param string $to * @param string $subject * @param string $body * @param string $from */ public function send($to, $subject, $body, $from) { mail($to, $subject, $body, "From: $from"); } } // ---------------------------------------------------------------------------------------- // // ---------------------------------------------------------------------------------------- interface UserRegistration { /** * @param $to * @param $username * @param $password * @return mixed */ public function register($to, $username, $password); } // ---------------------------------------------------------------------------------------- // // ---------------------------------------------------------------------------------------- class StandardUserRegistration implements UserRegistration { /** * @var PDO the connection to the underlying database */ protected $database; /** * @var MailSubmissionAgent */ private $mailSubmissionAgent; /** * StandardUserRegistration constructor. * @param MailSubmissionAgent $mailSubmissionAgent * @param PDO $database */ public function __construct(MailSubmissionAgent $mailSubmissionAgent, PDO $database) { $this->mailSubmissionAgent = $mailSubmissionAgent; $this->database = $database; } /** * @param string $to * @param string $username * @param $password * @return mixed|void * @throws Exception */ public function register($to, $username, $password) { $raw_token = mcrypt_create_iv(16, MCRYPT_DEV_URANDOM); $encoded_token = bin2hex($raw_token); $token_hash = hash('sha256', $raw_token); $hashed_password = password_hash($password, PASSWORD_DEFAULT); try { $sql = ' INSERT INTO users (email, username, password, confirmation_key) VALUES (?, ?, ?, ?)'; $stmt = $this->database->prepare($sql); $stmt->execute([$to, $username, $hashed_password, $token_hash]); $subject = 'Confirm Email'; $body = "Click to activate account\r\n" . APPLICATION_URL . "/activate.php?k=$encoded_token"; $this->mailSubmissionAgent->send($to, $subject, $body, ADMIN_EMAIL_FROM); die(header("Location: ./login.php?confirm")); } catch(PDOException $e) { if ($e->getCode() == '23000') { $error[] = "Registration Failed"; $error[] = "Invalid Username or Email"; show_form_errors($error); } else { throw new Exception($e); } } // End Catch } } // ---------------------------------------------------------------------------------------- // TESTING // ---------------------------------------------------------------------------------------- require ('../config.php'); // create an instance of a class which implement MailSubmissionAgent $mailSubmissionAgent = new PHPMailSubmissionAgent(); // create a registration object, passing the mail instance to the constructor $userRegistration = new StandardUserRegistration($mailSubmissionAgent, $pdo); // now the registration object can use the mail object $userRegistration->register('user@example.com', 'username', 'password');

I know you can't call an Interface directly (Instantiate). In the following code you call the interface directly. Is this "Dependency Injection"? And is the text MailSubmissionAgent type hinting? public function __construct(MailSubmissionAgent $mailSubmissionAgent) { $this->mailSubmissionAgent = $mailSubmissionAgent; } Going with your interface MailSubmissionAgent would this be a correct alternative implementation of the interface? class PHPMailSubmissionAgent implements MailSubmissionAgent { public function send($from, $to, $subject, $body) { mail($to, $subject, $body, "From: $from"); } }

After some digging, isn't my previous post referring to SOLID and the mantra "A class should only do ONE thing and do it really well"?

Are we talking about a flow like this where each part (validate fields, insert record, generate token/insert token, email token) is completely a separate interface/class and knows nothing whatsoever of anyone else? (Quick throw together) <?php if ($_SERVER['REQUEST_METHOD'] == 'POST'){ $validate = new ValidateRegistration(); if ($validate->validate === false) { // Show errors } else { // Attempt insert $register = new MysqlUserRegistration($pdo); if ($register->register === true) { $genToken = new TokenGenerator(); $token = $genToken->generateToken; $email = new SendBasicEmail(); $email->send('smith@example.com', 'Success', 'Registered. Your token is $token', 'from@example.com'); } } }

In reality I would never put the personal data in a users table, that would go in a person table. I was just trying to keep this simple while I am learning. I would like to tackle one problem at a time, emailing token on successful registration. Updated attempt: Renamed registration interface Removed personal information (first/last name) Created new email interface. Removed old send email function call Renamed registration class to a specific type name (Mysql) Question: Is there any issue naming a method the same name as the class? (class SendBasicEmail, function sendBasicEmail) Where I am stuck is integrating the additional emailing interface into the registration class. (Assuming I am heading the right direction.) Am I now forced to use namespaces and keyword 'use'? interface UserRegistration <?php /** * Register new user */ interface UserRegistration { /** * Insert user registration data * @param string $email Unique email * @param string $username Unique Username * @param $password * @return * @internal param mixed $hashed_password Hashed Password * @internal param mixed $token_hash Hashed token */ public function register($email, $username, $password); /** * @param $to * @param $from * @param $subject * @param $message * @return mixed */ public function sendBasicEmail($to, $from, $subject, $message); } class MysqlUserRegistration <?php class MysqlUserRegistration implements UserRegistration { /** * @var PDO the connection to the underlying database */ protected $database; /** * Connection to underlying database * @param PDO $database */ public function __construct(PDO $database) { $this->database = $database; } /** * @param string $email * @param string $username * @param $password * @throws Exception */ public function register($email, $username, $password) { $raw_token = mcrypt_create_iv(16, MCRYPT_DEV_URANDOM); $encoded_token = bin2hex($raw_token); $token_hash = hash('sha256', $raw_token); $hashed_password = password_hash($password, PASSWORD_DEFAULT); try { $sql = ' INSERT INTO users (email, username, password, confirmation_key) VALUES (?, ?, ?, ?)'; $stmt = $this->database->prepare($sql); $stmt->execute([$email, $username, $hashed_password, $token_hash]); $subject = 'Confirm Email'; $message = "Click to activate account\r\n" . APPLICATION_URL . "/activate.php?k=$encoded_token"; sendBasicEmail($email, $subject, $message);// Problem - From interface ProcessEmail die(header("Location: ./login.php?confirm")); } catch (PDOException $e) { if ($e->getCode() == '23000') { $error[] = "Registration Failed"; $error[] = "Invalid Username or Email"; show_form_errors($error); } else { throw new Exception($e); } } // End Catch } } interface ProcessEmail <?php /** Process Email */ interface ProcessEmail { /** * SendEmail constructor. * @param $to * @param $from * @param $subject * @param $message */ function sendBasicEmail($to, $from, $subject, $message); } class SendBasicEmail class SendBasicEmail implements ProcessEmail { function sendBasicEmail($to, $from, $subject, $message) { mail($to, $subject, $message, "From: $from"); } } Testing require('../config.php'); $test = new MysqlUserRegistration($pdo); $test->register('smith@example.com', 'myusername', 'mypassword');

Exactly. I first named it UserRegistration and then thought about what you had said before about the interface not being a specific implementation. Such as, this could be a user reg, a customer reg, a vendor reg, a company reg...etc which would be handled by the classes that implement it. e.g class UserReg implements Registration class CustomerReg implements Registration class VendorReg implements Registration class CompanyReg implements Registration Why is the thinking different in this case? I don't know the "how" of this. When you say component, do you mean another interface or class? I was aware something would need to be done with at least the emailing and error handling. I did what my current understanding allowed me to do. Is $additional_data somehow able to represent more than just one piece of additional data? That is exactly what I did since I do not know what to do yet. This project is specifically for learning the optimum OOP. The scope is currently limited to Registration, Login, Forgot Password, and Password Reset. From here I would need to see examples. I don't know how implement the things you mention. I can generally understand code much, much better than explanations. Thank you for your guidance!