benanamen

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!

The following is my working attempt at a user registration Interface. I am looking for feedback of what may be done wrong, documentation problems/improvements and any naming improvements. <?php /** * Register new user */ interface Registration { /** * Insert user registration data * @param string $first_name User first name * @param string $last_name User last name * @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 */ function register($first_name, $last_name, $email, $username, $password); } /*****************************************************************************************/ /*****************************************************************************************/ class UserRegistration implements Registration { /** * @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; } public function register($first_name, $last_name, $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 (first_name, last_name, email, username, password, confirmation_key) VALUES (?, ?, ?, ?, ?, ?)'; $stmt = $this->database->prepare($sql); $stmt->execute([ $first_name, $last_name, $email, $username, $hashed_password, $token_hash] ); $subject = 'Confirm Email'; $message = "Click to activate account\r\n" . APPLICATION_URL . "/activate.php?k=$encoded_token"; send_user_email($email, $subject, $message); 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 } } /*****************************************************************************************/ /* TEST /*****************************************************************************************/ require('../config.php'); $test = new UserRegistration($pdo); $test->register('Sam', 'Smith', 'smith@example.com', 'myusername', 'mypassword');

Anybody have any recommendations for an SRS/Use Case application?

With only 5 files in my class directory that have been recently created I am having to take a moment to think to remember which one is the Interface. As mentioned, you are not always going to be using an IDE when viewing those files. Even the manual shows a designator. It is also a standard in major frameworks. For those reasons and probably more, I will probably designate either a prefix or suffix. While I prefer the full suffix Interface for clarity, a prefix would be best for sorting. * I see that PhpStorm clearly shows what is an interface as @Jaques1 mentioned. That is not my prefered IDE at the moment. Perhaps I will switch which does solve the "problem" aside from viewing on github.

You make it easy for me to grasp this. So would this be the same concept? 1. Client says my DB has to do A,B and C - (Specification=Interface) 2. I create a DB schema (Concrete Blueprint = Class ) 3. The DB (Product manufactured = instance according to blueprint/class)

I saw this image representing an Abstract Class example and something clicked. Would it be correct to liken an Abstract Class and even a Interface to DB normalization? This image sure looks like it. Source http://www.zentut.com/php-tutorial/php-abstract-class/

That makes way more sense then everything I have read about it in the last two days.

I read the manual and numerous other resources on Abstract classes. Abstract classes appear to be very similar to Interface classes. When should I use an Abstract Class instead of an Interface Class? Should ALL classes that are not implementing an interface be programmed against an Abstract Class? If not, what decides that a class should not be derived from an Abstract Class nor an Interface Class?

On a quick review it looks like you did well. I pretty much never see that on a forum. I did notice you changed the id naming format in Reserva for the id_equip. I would add in the full name id_equipamento. it is the only column you didn't do it on.

That makes more sense to me but my OOP experience is extremely limited at this point. That is also the same type of style I have used in naming database columns and header and footer files. E.g. name_first name_last url_facebook url_twitter page_header page_footer

Just the interface class or the concrete classes that go with it as well. Or do you just mean any class file period? That's interesting because that's the name you came up with that I adopted. Was it an ok name before the update to an interface or you just get a fresh look at it? (https://forums.phpfreaks.com/topic/302286-oop-code-review/?do=findComment&comment=1538061) Luckily, none of this is really unfortunate. This app is all strictly to learn OOP so anything can be changed at any time. This is exactly what I was considering in the naming of the Interface itself. I understood from the last few days that an Interface can have many implementations like you just mentioned. In light of the class now being an implementation I agree the name should be more specific as you have suggested. I took that to mean doing this. <?php /** * A login attempt logger */ interface LoginLogger { /** * Logs a failed login attempt * * @param string $username the attempted username to log */ function logFailedAttempt($username); /** * Logs a successful login attempt * * @param string $username the attempted username to log */ function logSuccessfulAttempt($username); }