Jump to content

Archived

This topic is now archived and is closed to further replies.

Andy-H

Just wondering if anyone would be kind enough to take a look at my classes

Recommended Posts

I'm writing a CRUD API and want to use MVC (for the first time), just wondering if someone could cast wise-eyes over it and let me know if there's anywhere I'm going wrong, code attached, thanks :)

18447_.zip

Share this post


Link to post
Share on other sites

Well there is not much to comment on. Why do you have both an abstract class and an interface for a controller? To declare every controller with:

 

class Index extends Abstract_Controller implements Interface_Controller

 

Is really cumbersome. Either implement the interface on the AbstractController or drop the interface all together and declare the method as abstract. I would also postfix your controllers with Controller so they do not conflict with library classes. Why does the AbstractController define a dbhandle? This should be injected or something.

 

Why do you not use a framework?

Share this post


Link to post
Share on other sites

At the same time as building this I am trying to get better acquainted with OOP, I figured the best way to do that would be to "roll my own", I used an interface rather than defining an abstract method because there is no behaviour in the method defined, and my interfaces always define a public method, I know I can define an abstract method without any body, but I thought I was using abstraction for the purpose it was intended and the same with the interface, as for why the interface is not implemented on the abstract class, an oversight on my part  :shrug:

 

 

The getDBH method was implemented on the controller, because I load controllers dynamically, so in order to in ject the dbh, I would have to inject it in cases where a database connection might not be necessary, whereas I can just call $dbh = $this->getDBH() where necessary, then inject that into my models, is there a better way?

 

 

I will update the abstract classes to implement the interfaces, thanks for taking the time to review this :)

 

 

And thanks in general, you've helped me out a lot recently :)

Share this post


Link to post
Share on other sites
I would have to inject it in cases where a database connection might not be necessary

 

I meant injecting into the models (don't know why I forgot that part!) using a DI container, but if you don't feel up to using any third-party, and don't want to write your own, then yeah getDBH() will do.

Share this post


Link to post
Share on other sites

×

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.