4
\$\begingroup\$

I have the following table called info:

info_name   |   info_value

name        |   Susan

desc        |   Human

I'm trying to print Susan without knowing that the value is Susan.

The following code works, but I want to know if what I wrote is correct.

I'm open to any improvements.

Config Class:

class Config {

    private $hostname = 'localhost';
    private $username = 'blah';
    private $password = 'blah';
    private $database = 'blah';

    public function __construct()
    {
        $this->connection = new mysqli($this->hostname,$this->username,$this->password,$this->database);

        if($this->connection->connect_errno) 
        {
            die('Error: ' . $this->database->error);
        }
    }

    public function query($query)
    {
        return $this->connection->query($query);
    }

    public function __destruct()
    {
        $this->connection->close(); 
    }

}

Name Class:

class Name {

    protected $name;

    public function __construct()
    {
        $this->db = new Config;

        $r = $this->db->query('SELECT info_value FROM info WHERE info_name="name"');

        while($row = $r->fetch_row())
        {
            $this->name = $row['0'];
        }

    }

    public function getName()
    {
        return $this->name;
    }

}

Print Results:

<?php $show_name = new Name(); echo $show_name->getName(); ?>

Thanks for any help!

\$\endgroup\$
1
  • \$\begingroup\$ I think there's a typo in the error handling when the connection fails. $this->database->error will not work. Maybe you meant $this->connection->error. \$\endgroup\$ Commented Mar 28, 2012 at 23:07

2 Answers 2

3
\$\begingroup\$

Your config and name classes are good starts. Here are a few humble suggestions:

  1. I highly suggest steering clear of what is called the entity-attribute-value pattern when structuring your MySQL tables. Here is an article that discusses ways of refactoring. Also, if you are interested in learning more about mysql (and sql in general) I highly recommend the book SQL Antipatterns by Bill Karwin.

  2. In the future, if your projects grow and you want a more robust system, you may want to look into a few concepts such as dependency injection, which will help if and (hopefully) when you ever start unit testing with software like PHPUnit.

Hope this helps!

\$\endgroup\$
0
0
\$\begingroup\$

Well there is plenty of things you can do depending on what you want to archive!

Dependency Injection

as AndyPerlitch wrote dependency injection is one thing. creating

$this->db = new Config;

directly in your method is bad practice (from the dependency injection viewport) because you are stuck with config until you edit your code. If you want to change to Config2 or something else (like a mock for testing) you will have to go into this line (and every other line in your project) and change it! so

public function __construct($oConfig)
{
    $this->db = $oConfig;
}

would be an improvement

toString()

you could use this magic method to enable simpler output

toString() {
    return $this->name;
}

<? $oName = new Name($oDB); echo $oName; ?>

single responsibility

did you see how creepy it looks to pass the DB handler into the Name object? thats a good sign the name class shouldnt be doing things like querys , maybe you should seperate this kind of operations and use your name class for what it is an abstraction layer or container. you could pass a string into the __constructor and set a default value

   __construct($sName = 'john') {}

   $sName = $oConfig->query("SELECT...")->fetch_assoc()['name'] // this will not work but you get the point 
   $oName = new Name($sName); echo $oName;

oh wait the query statement is just floating there, in the middle of our script! lets put it in a class so we can reuse it easaly! we could make a CRUD class or a model or what ever the point is to seperate it from Name and make it reusable

class NameCRUD {
    // this is a good place for the db handler
    __construct($oConfig) {
        $this->db = $oConfig;

    }

    public function getName() {
        // thats your query function
   }
}

setter / getter

for some people it is good practice to use this methods to set and get properties, you should read about it and make your own choice.

and so on

you see there are so many things you can do, you have to ask yourselve if you need this kind of stuff. You will not need an abstraction layer for mysqli if you never ever will change from mysqli to somethnig else.

dependency injection is mostly for unit testing (there are ofcourse other benefits too) , but if you will not use phpunit .. well you get the point dont you? use what ever suits your project. Read some patterns or even more important some anti patterns and deicde for yourself. i hope i could help you a bit (- ;

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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