2
\$\begingroup\$

The active column in the database only contains a boolean. The country column contains ISO 3166 A2 codes (2 char). Do you have any advice on improving this class?

<?php
class WebShops {
    private $database;

    public function __construct() {
            $this->database = new Database();
    }

    public function fetchAll() {
            $this->database->query("SELECT * FROM web_shops");
            return $this->database->fetchAll();
    }

    public function fetchActive($option) {
            $this->database->query("SELECT * FROM web_shops WHERE active = :option");
            $this->database->bind(':option', $option);
            return $this->database->fetchAll();
    }

    public function fetchCountry($country) {
            $this->database->query("SELECT * FROM web_shops WHERE country = :country");
            $this->database->bind(':country', $country);
            return $this->database->fetchAll();
    }
}
\$\endgroup\$
2
\$\begingroup\$

You could validate $option and $country before you put them in a query. One needs to be true or false, and the other should be two letters A-Z. You need some kind of error handling for that.

For the rest: This is such a simple class, and the context is missing, what would you want us to say?

\$\endgroup\$
2
\$\begingroup\$

I agree with @KIKO Sotfware that becuase the question lacks context, we cannot provide an extensive review. But there is one thing which caught my eye.

You ahould utilize dependency injection in your constructor. By hard coding your database class into the constructor you make it hard to swap the implementation if a another one is required in the future or you want to write a test.

I would build a generic interface which implements the different method the class uses.

interface GenericDatabaseInterface {

    public function query($sql);
    public function bind($key, $value);
    public function fetchAll();

}

Your database class should implement this interface. You would then type-hint this interface in your constructors argument list. If you in the future requires another implementation the new class should also implement the interface.

public function __construct(GenericDatabaseInterface $database) {

    $this->database = $database;

}

Hope this can guide you a little, happy coding!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.