1
\$\begingroup\$

I have made a Query Builder, (not the full version of it yet) but I wanna ask if there is someone out there, there can improve this code even more. Will be nice to know so I can code the rest in the better way :)

<?php
class DB {

    private static $instance = NULL;

    public static function sql(){
        self::$instance = new self();
        return self::$instance;
    }

    // These methods set the query clauses
    // Use * as default
    public function select(string $select_options = "*") {

        $query_part = "SELECT ".$select_options;
        $this->select = $query_part;
        return $this;
    }

    // From table in DB
    public function from(string $from_options) {

        $query_part = "FROM ".$from_options;

        $this->from = $query_part;
        return $this;
    }

    // These methods set the query clauses
    public function where(string $where_options) {

        $query_part = "WHERE ".$where_options;
        $this->where = $query_part;
        return $this;
    }

    // GroupBy
    public function groupBy(string $group_options) {

        $query_part = "groupBy ".$group_options;
        $this->groupBy = $query_part;
        return $this;
    }



    public function get() {
        $array = (array)$this;
        $string = implode(" ", $array);

        return $string;
    }

}

And this I how it to call the query builder

// Use class DB
$query = DB::sql()
    ->select()
    ->from("CustomerInfo")
    ->where("customer_id > 300")
    ->get();

// Echo result
echo $query;
\$\endgroup\$
  • \$\begingroup\$ "not the full version of it yet," So you're asking about completing that code? (which may turnout to be off-topic) \$\endgroup\$ – πάντα ῥεῖ Jan 3 '17 at 19:49
  • \$\begingroup\$ @πάνταῥεῖ No! Im only ask for if the code i view to you can be improved \$\endgroup\$ – TheCrazyProfessor Jan 3 '17 at 19:51
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Mast Jan 5 '17 at 9:22
  • \$\begingroup\$ @Mast oh, sorry. I didn't knew that :( \$\endgroup\$ – TheCrazyProfessor Jan 5 '17 at 9:29
3
\$\begingroup\$

Layout for code: Look at PSR spec: http://www.php-fig.org/psr/psr-2/

for comments: https://www.phpdoc.org/docs/latest/getting-started/your-first-set-of-documentation.html#writing-a-docblock

Finally is your code testable and possibility to extend without modifying it?

Also "(static) $instance" doesn't feel right to me for some reason, try different name?

\$\endgroup\$
  • \$\begingroup\$ Yes the code work 100% without any need for modifying. I will improve the comments and add more, for now it's just so I quick can navigate around. the "instance" any ideas for a new name? \$\endgroup\$ – TheCrazyProfessor Jan 3 '17 at 20:11
  • \$\begingroup\$ And thanks for the links, I will look at them :) \$\endgroup\$ – TheCrazyProfessor Jan 3 '17 at 20:12
  • \$\begingroup\$ Instance, I'm thinking object instance and static usually means not related to an instance of an object... \$\endgroup\$ – user127379 Jan 4 '17 at 20:21
1
\$\begingroup\$

This class frankly seems pretty trivial and would have a long way to go to get to something useful. I would agree with thoughts in other answer around adopting a consistent style.

I don't understand the static / concrete duality of this class. Why the need for the static instantiating method at all?

The biggest problem here though is that this code is incredibly fragile, and here is where you would need to put in most effort to get towards a class the really brings value over simply writing queries manually.

There's nothing to validate input to your various methods is suitable to produce a valid query.

There's nothing to enforce that these methods are called in the proper order, and you concatenation approach simply hopes that the order of properties when cast to array are proper.

You don't even specify the properties in the class, instead defining properties on the fly when methods are called, so there is nothing to enforce that all properties even exist on he object.

You lose flexibility in writing your queries. Unless you want to build a significantly more complex set of methods on this class, you can really only build the most basic queries using this class.

And you introduce all these problems into your code for what? I don't see what value this class brings versus to writing out the queries.

Just write the queries. They would be easier to read and maintain.

\$\endgroup\$
  • \$\begingroup\$ it don't have validaton, for it's only the coder there can use it. and so are it a project for fun, sooooo.. \$\endgroup\$ – TheCrazyProfessor Jan 5 '17 at 8:53

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.