Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework the query builder and make it more robust #7

Open
isocroft opened this issue Jan 11, 2017 · 6 comments
Open

Rework the query builder and make it more robust #7

isocroft opened this issue Jan 11, 2017 · 6 comments

Comments

@isocroft
Copy link
Owner

isocroft commented Jan 11, 2017

Draw inspiration from https://github.com/usmanhalalit/pixie

@shalvah
Copy link

shalvah commented Jan 23, 2017

Couldn't we possibly just integrate that one?

@isocroft
Copy link
Owner Author

isocroft commented Jan 23, 2017

Hmmmm... I did have a battle with whether or not to go integrate fully or just implement using its' idea. Here's my thought flow on why i am considering not integrating (Please, let me still have your thoughts):

  1. I feel that Jollof should have a very different syntax (might be simpler than pixie) signature for setting up queries in Jollof that looks like the below
       /* This will make a query to a table registered by the model for all columns in rows where `age` column is less than 35 with a LIMIT of 10 and an OFFSET of 5 in descending order */
       $userModelInstance->get(array('*'), array('age' => array('<', '35')))->ordering('age', FALSE)->exec(10, 5);
      /* This will make an insert query to a table registered by the model and UPDATE ON DUPLICATE KEY (id) the `role` column to 1 */
       $userModelInstance->set(array('id' => 'USR29839200299393','name' =>'henry', 'gender' => 'male'), array('role' => 1))->exec();
  1. I feel that Jollof does not need a table( ) method in the query builder object since these methods are called from the model object (as above).

Let me know what you think...

@isocroft
Copy link
Owner Author

isocroft commented Feb 15, 2017

Also @shalvah, The repo has some bugs and dire needed enhancements which the project maintainer has not attended to. A whooping total of 28 issues on the issues tab. I think that's quite a lot if you ask me. This is another reason why I can't just integrate it.

I have already read the code in the repo and what i am currently doing is to migrate the core of code (with a few changes) to Jollof.

I believe this is faster. Please, if you think you better ways to achieve let me know.

@isocroft
Copy link
Owner Author

@shalvah , i have completed the integration for the pixie query builder project code. also added support for MongoDB (though unstable for now). please test it out and give me feedback

@shalvah
Copy link

shalvah commented Mar 21, 2017

Sorry, I've been quite occupied. Reading the code above now, I feel those methods should be static methods, since they are actually not tied to specific users, for instance. Also, the need for a table function arises when someone wishes Tim perform a "manual" database query which isn't tied to any model class.

@isocroft
Copy link
Owner Author

isocroft commented Mar 21, 2017

Yes, you are correct. I have made static methods of these. The base Model class now carries static methods that call these (protected methods). Other model classes extend the base model class to inherit the static classes. So, use is as follows (example):

    // you have...

     TodosList::whereBy(array('user_id' => '=', 'xxxxxxxxxxxxxxx')); 

     // instead of

     $todoModelInstance->get(array('*'), array('user_id' => '=', 'xxxxxxxxxxxxxxx'))->exec(0, 0); // the {get} method here is protected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants