patternphpMinor

PHP library for handling account creations, logins, and file uploads

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

uploadshandlingfilecreationsphploginsaccountforlibraryand

Problem

I'm new to OOP and PHP. I've made an attempt at a PHP library file that handles account creations, logins, and file uploads with image resizing on the fly. It works so far. I'd like some help with the Obj Oriented part, namely making the code more efficient/succinct.

User class:

createNewUser();

        $userpath = BASEPATH . '/users/user_' . $userID . "/";
        $userFiles->userDirectory( $userpath );

    }

    /*END CREATE ACC*/

    /**
     * @param $postVars
     * @return bool
     */
    function loginUser( $postVars ) {
        $newLogin  = new DB();
        $user_data = $newLogin->findUser( $postVars );
        $session   = new userSession( $user_data );

        return true;
    }//**END loginUsr function

    /*function userLogOut{
     1. take $userData and session info and unset session
     2. redirect to homepage
     unset($_SESSION);

    }*/

}

Render class:

homePath   = AppRoot;
        $this->stylePath  = $this->homePath . '/style.css';
        $this->javascript = $this->homePath . '/eventsJS.js';
        $this->beginHTML  = '';

        $this->headTag = 'EventfinderstylePath . '" type="text/CSS"
       rel="stylesheet" /> 
       
        javascript . ' "> ';

        $this->headerDiv = " Eventfinder ";
    }

    function index() {
        echo $this->beginHTML;
        echo $this->headTag;
        echo $this->headerDiv;

    }

    function usrLogin() {
        $newform = new html_;
        $newform->userLogin();

    }

    function createAcc() {
        $forms = new html_;
        $forms->newUserForm();

    }

}

HTML Class:

```
'button', 'checkbox' => 'checkbox', '_file' => 'file', 'hidden' => 'hidden', 'radio' => 'radio', 'r_set' => 'reset', 'submit' => 'submit', '_text' => 'text' );

public $formOpen = '';
public $formClose = "";

public $inputText = '';

/**
* @param $method
* @param $action
* @param $formName
*/
function openForm( $method, $action, $formName ) {

Solution

Executive summary:

Pick a naming standard and stick to it (img != image for instance)

Learn how to write comment blocks ( / description / / @var */ is kinda weird)

SOLID is a good place to start (google it)

PSR-1 and PSR-2

Don't store html in strings, just don't.

Again, don't use a class to represent a 'template'. Php does a really good job as a templating engine `




your database isn't really abstracted (more on this later)



I miss interfaces :( (why? I just like them)



So why bother commenting?
 - You did a good effort in separating concern
 - you actually wrote some comments
 - You are using PDO 

Long part

(More text will follow, I simply don't feel like writing everything in 1 go):

FileHandler or no, fileHandler

Naming convention is everything . When using code, you should not have to think about syntax. It should all feel very natural. Your code doesn't. Some examples:

the

FileHandler class is actually fileHandler

. Always start your class names with a capital letter. Why? because. It's a convention, we all do it, it makes writing code easy.

Now, lets say I want to use your fileHandler class. I copy paste it into my project and initialize it:

$myAwesomeStolenFileHandler = new fileHandler();
$myAwesomeStolenFileHandler->createImage($myImage);


OW sh*@t, function not found. Huh? but it's there in the code allright. Ow, wait, it's not a method of the class, it's a function inside a method. I'm ot supposed to use it, so why not make it private? Instead of creating it everytime I call createImage. wow, thats enoying. uh ok, well, I was planning on adding a watermark but i'll wate for SteveBK to implement it I guess. Ok, well lets use the imageresizer then.

$myNotSoAwesomeStolenFileHandler->resizeImage($image);


Again, method not found.

Huh? confusion

Aaah, it's

img not image. and in this class somehow it's imgResize and not resizeImage

.

Ok, next try;

$myNotSoAwesomeStolenFileHandler->imgResize($image);


Note that I had to read the method itself to be able to know what

@param $image

 actually is (is a String? is it a binary object, is it, ...)

Recap:

Comments aren't really helping



A lot of functionality is cramped into one method (and those strange function creations on the fly in a method, iieeewww)



naming of the methods could be a lot better.



Rewrite:

What does the name FileHandler tell me? Well, without reading the code it tells me that it's a class that aids me in handling files. I could for instance want to get a list of files. Add a file, upload one, rename it, get a FileInstance to do some extra stuff with the file itself, ... You know, the usual stuuf a FileHandler does. it would look something like this:

<?php
interface FileHandler
{
    /**
     * Define the root path for the FileHandler
     * The FileHandler should not be able
     * to access files outside this root folder
     * 
     * @param String $root the root path
     */
    public function __construct($root);

    /**
     * Returns an array of all file and directory names in the current directory
     * IF $showDirectries == false
     *  exlude the directories
     * 
     * @param  boolean $showDirectories
     * @return String[]
     */
    public function listAll($showDirectories=true);

    public function changeDirectory();

    public function getCurrentDirectory();

    public function getFile($name);
}


Yes, Id din't finish all the comments. Complains can be written to /dev/null

Interface?

Yes, I wrote an interface and not a class. why? here is the reason.
An interface is a contract, It tells us programmers what a certain class/object can do if it implements this interface. This also goes the other way round, if we need an object of a certain interface. we simply type-hint for that interface and we get really nice error messages that help us in the debug proces (for instance if someone is trying to cramp a MouseHandler to your app instead of a FileHandler)

Back to the FileHandler that actually should have been a ImageInstance or ImageFile

As you see, I now wrote a piece of code that has nothing to do with your app, simply because you didn't need a FileHandler. You need a ImageResizer, or whatever you need.

Handling independent files and/or images

Our FileHandler has a method

getFile()

 and let's for the sake of codereview and my essay I'm writing here go all the way and have it a return an object of type

FileInstance`.

The FileInstance

A FileInstance is a representation of a file. Lucky for us, we don't have to write this ourself, there is a really good built int class for this, the SplFileInfo class

What about my ImageResize

good question, the ImageResizer is a function/method that needs a FileInstance and then does some really cool stuff to it (like resizing). The ImageResizer needs a File or a link to a file. We us the SplFileInfo object here.

ImageResizer inteface

Here we can go for different approaches, I will go for

Code Snippets

$myAwesomeStolenFileHandler = new fileHandler();
$myAwesomeStolenFileHandler->createImage($myImage);

$myNotSoAwesomeStolenFileHandler->resizeImage($image);

$myNotSoAwesomeStolenFileHandler->imgResize($image);

<?php
interface FileHandler
{
    /**
     * Define the root path for the FileHandler
     * The FileHandler should not be able
     * to access files outside this root folder
     * 
     * @param String $root the root path
     */
    public function __construct($root);

    /**
     * Returns an array of all file and directory names in the current directory
     * IF $showDirectries == false
     *  exlude the directories
     * 
     * @param  boolean $showDirectories
     * @return String[]
     */
    public function listAll($showDirectories=true);

    public function changeDirectory();

    public function getCurrentDirectory();

    public function getFile($name);
}

<?php
interface ImageResizer
{
    /**
     * Constructor, set some defaults
     * @param int $width           the width of the image
     */
    public function __construct($width);

    /**
     * Resize the image according to a set of rules defined in the __construct
     * 
     * @param  SplFileInfo $image the image to resize
     * @param  SplFileInfo $destination the destination for the resized image
     * @return void
     * @throws MymeTypeNotSupportedException If myme_type<$image> is not supported
     * @throws DestinationFileNotWritable If !$destination->isWritable()
     */
    public function resize(SplFileInfo $image, SplFileInfo $destination);
}

Context

StackExchange Code Review Q#52363, answer score: 7

Revisions (0)

No revisions yet.