panique / mini2

Just an extremely simple naked PHP application, useful for small projects and quick prototypes.
MIT License
416 stars 102 forks source link

[moved to MINI3] example for multiple "models" and passing data between them #25

Closed panique closed 8 years ago

panique commented 9 years ago

taken from php-mvc

dvdmierden commented 9 years ago

Hi!

Was searching for a solution for this, and found this placeholder :-)

I'm currently just loading another model like so:

class actorModel extends Model {

function search() {
$movie = Controller::loadModel('Movie');
//..
}

This used to "work", but only if I called the search() method from the controller like this:

class Actor extends Controller {
function search() {
$movie = $this->loadModel('Movie');
$actor = $this->loadModel('Actor');
$actor->search();
//..
}

Unfortunately, recently my PHP version was upgraded from 5.x to 5.4.34 ... and the above code now triggers E_STRICT errors:

Strict Standards: Non-static method Controller::loadModel() should not be called statically, assuming $this from incompatible context in

I'm a total n00b at MVC coding. Basically, in my ActorModel, I need MovieModel methods. And I have no idea how to correctly do that. Any thoughts greatly appreciated!

Cheers, Daniel

VeeeneX commented 9 years ago

@dvdmierden I suggest to use DI (Dependency injection)

panique commented 9 years ago

[sorry, I just posted into the wrong repository]

ghost commented 9 years ago

I have been using composers autoloader and Namespaces to get around the problem. Then I have just been calling the class as I would normally.

dvdmierden commented 9 years ago

That DI does sound like a solution.

Let's see if I get this straight: Is it similar (or actually exactly like Panique does with the NoteModel?)

class NoteModel
{
    /**
     * Constructor, expects a Database connection
     * @param Database $db The Database object
     */
    public function __construct(Database $db)
    {
        $this->db = $db;
    }

So basically, I could write the following code ?

class ActorModel extends Model {
  public function __construct(Database $db, Movie $movie) {
    $this->db = $db;
    $this->movie = $movie;
  }
  //..
}
VeeeneX commented 9 years ago

@dvdmierden Yes, That's right, but Movie must be an instance of Movie, that makes sense and you have to add that property (movie) inside of ActorModel, then you have to edit following lines: in application/libs/controller.php

public function loadModel($model_name, $DI = false){

    require 'application/models/' . strtolower($model_name) . '.php';
    // return new model (and pass the database connection to the model)
    if($DI){
        return new $model_name($this->db, $DI);
    }
    return new $model_name($this->db);
}

That should work, I haven't tested it yet.

dvdmierden commented 9 years ago

@VeeeneX Wowo, thanks for the quick reply! I guess indeed that could work... but somehow it seems .. dirty. Especially since I'll probably have more dependencies.

And what If I have some Movie method that depends on Artist methods and vice versa? It feels like this is the wrong approach..

panique commented 9 years ago

He @dvdmierden , when I'm using multiple models, I'm always doing this in the controller, like

$songs_model = $this->loadModel('SongsModel');
$stats_model = $this->loadModel('StatsModel');

$songs = $songs_model->getAllSongs();
// pass something from A to B
$xxx = $songs_model->doSomethingWithTheSongs($songs);
dvdmierden commented 9 years ago

@panique Yeah, That's how I started too .. but what if in your example method getAllSongs() you would also get the Artist, for each song?

ghost commented 9 years ago

I was looking at PHP traits but chosen to extend the class to share methods

class ActorModel extends MovieModel
{
public $db;    

public function __construct($db)
{ 
    parent::__construct();
    $this->additionalMethod();
}

public function actorMethod()
{
    parent::movieMethod();
        //more stuff...
}

private function additionalMethod(){...}
}

$actor_model = $this->loadModel('ActorModel');
$actor_model->actorMethod();

interested as to why/why not to do this.

panique commented 9 years ago

@dvdmierden The songs / stats thing is just a superbasic example, in this case I would write a method like getAllSongsWithArtist() in the songs model OR request the artist for each song via something like getArtistOfSongBySongId($song_id) [which could be called from the controller]. This might requests a lot of DB calls, but totally works for simple applications.

I'm a fan of doing things extremely simple.

dvdmierden commented 9 years ago

@panique That makes sense. The getAllSongsWithArtist() way is what I'm actually used to, but some cocky recruiter called that "junior" coding.. whereas I'm trying (to learn) to write the right way

This is actually the main reason for me using/learning with your framework :-)

panique commented 9 years ago

To be honest, this "framework" (which is just a supersimple boilerplate) does MVC wrong (but still in the same way lots of tutorials and other frameworks do). I wasn't aware of that and will rename and rewrite this script in the next time...

So, please don't rely on this mvc approach too much :)

dvdmierden commented 9 years ago

I've checked out a lot of these "frameworks" but most of them were way too elaborate to get started with. I liked the simplicity & readability of yours and while it may be 'wrong', it does help me get the basics of it.

Having said that: if you're ever going to release a "proper" MVC framework, I'm a fan ;-)

panique commented 9 years ago

@dvdmierden Big thanks! :) Feels very good to hear this.

The big issue with php-mvc is: This project has (beside lots of positive feedback) been bashed in very rude, hurtful, unconstructive way by lots of people, because it does not follow rule X, does MVC wrong, is stuck behind all other "frameworks", has security issues etc. Some of the bashers are big people in the PHP world, so this is not totally irrelevant.

Basically everything they say is true, but they don't get that this is just a tiny little script that I've published on GitHub as it has worked out for me (and some dev friends) very very well, and obviously lots of people thought the same. The (totally unintended) popularity of the script, the SEO-friendly name and the fact that I've written things like "perfect to learn how MVC works" inside makes lot of people angry, as this is not what php-mvc does.

To come back to what I want to say: I'm currently in the process of "removing the entire project from the web" (so nobody gets a wrong idea of MVC and nobody has a reason to complain), renaming it (to "MINI") and rewriting lots of stuff and finally re-releasing this with upgraded code and additional features. So if you have any ideas or wishes what could be inside the script, then please send me a mail or comment here. :)

panique commented 9 years ago

@dvdmierden By the way, another supersimple approach regarding the models-issue could be this:

According to several articles the "model" is just a layer in the application, so using several files/classes and calling them Songs-model, stats-model etc seems to be wrong. But, this is exactly how nearly all frameworks do this (this article also confirms this).

To keep things extremely simple, it might be nice to simply use one simple file models.php that holds all the model-methods, like getAllXXX(). With this approach, it would also eliminate the communicating-between-models-issue.

Expert developer might find this horrible (?), but for smaller projects it might be totally okay.

Intereesting: http://php-html.net/tutorials/model-view-controller-in-php/

VeeeneX commented 9 years ago

@dvdmierden I would use Dice for DI then

dvdmierden commented 9 years ago

Hi guys,

Thanks for your input. Dice looks Nice. I'll see if I can experiment a little with it. For now, I've taken @panique 's advice and gone with the single model file. This works perfectly, but the file gets tediously long and my editor doesn't collapse functions. Ah well, can't have it all, I guess :)

panique commented 9 years ago

@all: You can find a real implementation of the model-in-one-file thing in MINI (which is the updated version of ex-php-mvc btw): https://github.com/panique/mini

This project here, php-mvc-advanced, will also be rebuild in the next week, will be like MINI, but with more features.

panique commented 9 years ago

FYI: this repo just had a fresh start, it's called MINI 2 now and in early alpha. Currently it uses just one model-file that holds all database-requests etc., so let's discuss how a more advanced version of this could look like.

tankerkiller125 commented 9 years ago

Mabey something very similar to the original php-login model handling system? I rather enjoyed that system and found it very easy to use (for most cases). If I get a chance this week I'll make a fork and start playing around with model loading.

panique commented 8 years ago

hey guys, this is now implemented in MINI3, please find and test it here :) https://github.com/panique/mini3