-2

My code looks like below and I want to clean it by removing repeated parts, such as mysqli functions. I've tried putting things outside the functions, but they don't work because of scope. I tried adding global but things mysteriously broke as well. What's the best practice for this?

public function get_thing($p, $is_project) {
        $db = mysqli_connect(localhost, user-name, password, database-name);
        $user_id = $_SESSION['user_id'];
        $result = mysqli_query($db, "SELECT id, task, parent, status, created_at FROM tasks WHERE user='$user_id'");
        while ($row = mysqli_fetch_row($result)) {
            print row[0];
        }
    }

    public function del_thing() {
        $db = mysqli_connect(localhost, user-name, password, database-name);
        $user_id = $_SESSION['user_id'];
        $result = mysqli_query($db, "SELECT id, task, parent, status, created_at FROM tasks WHERE user='$user_id'");

    }

    public function count_thing() {
        $db = mysqli_connect(localhost, user-name, password, database-name);
        $user_id = $_SESSION['user_id'];
        $result = mysqli_query($db, "SELECT id FROM tasks WHERE user='$user_id'");
        return mysqli_num_rows($result) . " tasks";
    }
halfer
  • 19,824
  • 17
  • 99
  • 186
Noah
  • 4,601
  • 9
  • 39
  • 52

5 Answers5

2

The important thing to keep in mind is that there are a lot of established solutions to your question. One of the most effective tools is to utilize class structure with Class Injections to share global variables. I would not do this structure justice and it took me a while to make my project based on classes. The benefits are numerous though.

  • With well documented classes and functions, you will be developing a robust set of tools which will help you create multiple applications with the same libraries. This allows for increased efficiency in coding

  • With well organized classes it will be extraordinarily easy to extend the power of PHP without killing yourself. Think about this as you might with PHP's built in classes, like mysqli. It is incredibly useful to have classes which are built to accomplish tasks you need to use multiple tasks.

  • With well structured code, you will be saving yourself an incredible amount of time in debugging. If you need to make a change, you will only have to do it sparingly.

Some tips:

Use constants with a separate file, where your common files like file paths exist.

Creating a config file is a great way to limit the number of times you have to type the same value over and over again, helping to fulfill one of the most important principles of coding, Don't Repeat Yourself (DRY). That means your config file can keep some common values such as:

  • Your SQL username, dbname, password.
  • Global values that do not need to change. For example, if you know that BLUE will always have the value 1, you can define('BLUE',1);

Document your code well.

I fell into this trap multiple times, and I think most coders have come to the realization at some point in their lives that they are complicating their lives simply by failing to comment. A well structured program will have classes that have functions within them that make perfect sense. But you won't really remember what those are unless you comment each class's purpose and each function within them. A great outcome of this is that you can actually describe what your functions are doing before you code them. It makes it a lot easier to make sure you are not duplicating efforts or not creating two functions where one will do/one function where two will do.

Utilize class hierarchy.

Utilizing smart class hierarchies can help you accomplish some very sleek and powerful code. Using bad class hierarchies can help you give yourself a nightmare of dependencies. For example, many of your classes will probably be calling SQL. If you are utilizing Dependency Injection then you will need a way to inject your code. Creating an abstract class called base can help to make that happen.

/**
 * Base classes for dependency injection.
 */
Abstract class base {
    protected $db,$session,$logged_in;

    /**
     * Sets the database $db handler.
     *
     * Classes which inherit this class can use $childClass->db
     * to utilize any SQL queries.
     *
     * @param object $db Database handler from mysqli_connect.
     */
    public function set_database($db) {
        $this->db = $db;
    }

    /**
     * Session injection.
     *
     * If you want to use dependency injection to its fullest,
     * avoid globals and use something like this or more complex
     * to store your sessions too.
     *
     * @param array $session from the $_SESSION global.
     */
     function set_session($session) {
         //You can store a variable to quickly check if the user is logged in.
         if(!isset($session['logged_in'])) $this->logged_in = false;
         else $this->logged_in = $session['logged_in'];
         
         $this->session = $session;
     }
}

/**
 * The class where I do all my function execution.
 *
 * Because it extends base, I have the set_database function.
 */
class myClass extends base {
    /**
     * Delete something.
     *
     * @return integer The number of rows deleted.
     */
    public function del_thing() {
        $result = mysqli_query($db, "SELECT id, task, parent, status, created_at FROM tasks WHERE user='{$this->session['user_id']}'");
        return mysqli_num_rows($result);
    }
}

To call something like this, you have to execute a few extra lines of code.

//The old way
//$myClass = new myClass;
//$myClass->del_thing();

//The new way
$database = mysqli_connect(LOCALHOST, USERNAME, PASSWD, DBNAME);
$myClass = new myClass;
$myClass->set_database($database);
$myClass->set_session($_SESSION);
$myClass->del_thing();

The benefit of this method is that although you have to write a couple extra lines of code, you are then free to write your code freely. Additionally, you don't have to repeat everything multiple times. That means if you have to migrate databases, or change usernames, or anything, you make ONE change, and it is all set throughout. This helps to ensure your code is not tightly coupled, another big no-no in well crafted code.

A great way to solve any annoyance with having to declare set_database is to utilize Pimple, which is actually the same tool that is used in the Symfony Framework, developed as a standalone tool. It is a fairly simple tool to get up and running and now, instead of what was written above, you write, one time, in an initializing library of some sort:

$container = new Pimple;
$container['myClass'] = function($c) {
    $class = new myClass;
    $class->set_database($database);
    $class->set_session($session);
    return $class;
};

Now in the future, rather than calling $myClass = new myClass; to create a class instance, you can instead call $myClass = $container['myClass'];. From that point on, $myClass->del_this(); will work the same way.

Community
  • 1
  • 1
smcjones
  • 5,490
  • 1
  • 23
  • 39
  • A convincing case but how to take it into practice? – Noah May 17 '14 at 04:23
  • Look at Pimple, made by the same team that created Symfony. Also brush up on basic principles of dependency injection. It is all over the web. One tutorial will not cut it... Read at least four if you want a good answer. – smcjones May 17 '14 at 06:09
  • Thanks a lot for this detailed answer. I have not implemented it yet, but will revisit it in the coming weeks. Thanks again. – Noah Jun 11 '14 at 05:39
  • Struggling to understand this from my skill level, I'm afraid. – Noah Jun 12 '14 at 16:02
  • Fair enough. Hopefully some of this has helped you. And it doesn't hurt to revisit and struggle through it someday. The only way to learn. – smcjones Jun 13 '14 at 19:58
0

As you seem to be using classes, the first thing to do, is make a global database connection (only one...) and inject that in the current class (look up dependency injection).

Something like:

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

Then you can use $this->_db everywhere where you need it.

jeroen
  • 91,079
  • 21
  • 114
  • 132
  • Thanks. Where should I put this? I have one class called Thing, but nothing in it but functions. Where should I define $db? In the class ? – Noah May 17 '14 at 02:55
  • @Noah No, you open one global database connection and you pass that to the constructor when you create an object. See for example this question about dependency injection: http://stackoverflow.com/questions/2255771/how-can-i-use-dependency-injection-in-simple-php-functions-and-should-i-bothe – jeroen May 17 '14 at 02:56
  • Unfortunately I'm below that level of understanding, I think. – Noah May 17 '14 at 03:34
  • @Noah just think of it as sending the database as a parameter to the function. But using OOP and the constructor, you only have to do it once instead of in every function. – jeroen May 17 '14 at 03:49
  • Right. But how does that code do that? Where are the mysqli functions, query, etc? – Noah May 17 '14 at 04:22
0

Use singilton patters.

public function __construct(mysqli $db) {
    if (!isset($this->_db)) {
        $this->_db = $db;
    }

    return  $this->_db;
}
Dinuka Thilanga
  • 4,220
  • 10
  • 56
  • 93
-1

The answer seems to be here: https://stackoverflow.com/a/10208763/2183008

Create a new php file (eg. config.php, connect.php) and put the repeated things in there. Then use require_once 'config.php' in each function.

Community
  • 1
  • 1
Noah
  • 4,601
  • 9
  • 39
  • 52
  • That is never going to work as only the first time you run any of the functions, the file will be included so all other function calls will not have access to the database. – jeroen May 17 '14 at 03:09
  • They're not using `require_once` inside a function. But you can always give it a try :-) – jeroen May 17 '14 at 03:46
-2

Managed to save a lot of space this way.

class Database {
    public function query($query) {
        $db = mysqli_connect(localhost, user-name, password, database-name);
        return mysqli_query($db, $query);
        }
    }

class Things {
    public function get_thing($p, $is_project) {
        $result = Database::query("SELECT id, task, parent, status, created_at FROM tasks WHERE user=1");
        while ($row = mysqli_fetch_row($result)) {
            print row[0];
        }
    }

    public function count_thing() {
        $result = Database::query("SELECT id, task, parent, status, created_at FROM tasks WHERE user=1");
            return mysqli_num_rows($result) . " tasks";
    }
}

}

I'd appreciate feedback on this solution, if you have it.

Noah
  • 4,601
  • 9
  • 39
  • 52
  • 1
    You are missing a closing bracket. You also are going to have to create a brand new connection to `mysql` on each new class call. I have an alternative I will write up for you in my answer. – smcjones May 20 '14 at 00:28