1

I have 2 objects that are related. Album->Song. One album has many songs. Within the album class i have a method called GetSongs(). This does what you would expect.

I have a base class that contains generic functionality i use across all objects. Both album and song extend this object. The important method here is. 

public function Get(){
    return new static();
}

Both album and song have a method called ByID($postid). This method will take the ID, fetch the info from the DB and populate the blank object. This allows me to create an album like this. This code works fine.

$album = Album::Get()->ByID(1);

public function ByID($postid) {
    $post = "sql to get info from db";
    $this->Title = $post['title'];
    .....
    return $this;
}

The problem arises when I call a method called GetSongs(). For some reason this method is returning Album objects.

public function GetSongs()
{
    $songresults = "SQL and other code to get the songs from the db";

    $songs = array();
    foreach($songresults as $song) {
        $songs[] = Song::Get()->ByID($songresults["ID"]);
    }
    return $songs;
}

This seems quite simple, but i dont understand what is going on. Song::Get()->ByID() is actually calling the ByID method from the Album object and not the Song object, even though the code is referencing the song object.

Dan Hastings
  • 3,241
  • 7
  • 34
  • 71
  • 1
    I would think that as both static classes have the same method name that you should then put each class into a namespace, You'd need to read up about [PHP namespaces](http://php.net/manual/en/language.namespaces.php) , unfortunately It's not something I know too much about. – Martin Jun 17 '16 at 10:44
  • What's the point of your `Get` method over `(new Album)->byID()`...? – deceze Jun 17 '16 at 10:46
  • it is my understanding that you need to instantiate the class as static. http://stackoverflow.com/questions/5197300/new-self-vs-new-static – Dan Hastings Jun 17 '16 at 10:50
  • Feels like either your ByID methods should work as static factories, or you should just use a default constructor and then an initialisation instance method. The `get()` method really isn't adding anything. – iainn Jun 17 '16 at 10:52
  • @Dan Not sure what this is in response to, but if your `Get` method literally only returns a `new static`, it's pretty pointless and should be replaced with `new Album` instead of `Album::Get`. – deceze Jun 17 '16 at 10:52
  • 1
    Also... this *should* work as is; without seeing a full implementation of your `Song` and/or `Album` class it's impossible to say why you're instantiating the wrong class. – deceze Jun 17 '16 at 10:53
  • @deceze I want to chain methods together If i chain the ByID method onto new Album()->ByID(1) i get a syntax error. unexpected '->' – Dan Hastings Jun 17 '16 at 10:55
  • As I wrote above: `(new Album)->ByID(1)`. But really, it should probably really be `Album::byId(1)`, or *really really* `$albumStorage->byId(1)` (returning an `Album` instance). Your class structure isn't exactly optimal. Conflating SQL queries into business objects is sub-optimal. – deceze Jun 17 '16 at 10:56
  • @deceze ok this works, could you explain the significance of brackets in this case? – Dan Hastings Jun 17 '16 at 10:58
  • 1
    It's simply what's required to disambiguate the syntax for PHP and let it know that you want to call `byId` on the result of `new Album`, and not call `new` on `Album->byId`. (Arguably it's a stupid syntax restriction, but many parts of PHP are this way...) – deceze Jun 17 '16 at 10:59
  • @deceze ok thanks for this, very helpful. Appears this method of creating an object only works with php 5.4+ which is going to be an issue for me as this will likely get installed on machines with 5.3. This has fixed my problem for now though. i guess i might just have to settle for not chaining the methods. Thanks for the help, id give you the correct answer if you leave an answer – Dan Hastings Jun 17 '16 at 11:04

0 Answers0