Classes.... Right vs Wrong

#1

If I create a class called SampleClass, and give it a method called GetRecord, is there a difference between:

$test = new SampleClass();
$var1 = $test->GetRecord(1);
$var2 = $test->GetRecord(2);
$var3 = $test->GetRecord(3);
$var4 = $test->GetRecord(4);

And:

$var1 = new SampleClass();
$var2 = new SampleClass();
$var3 = new SampleClass();
$var4 = new SampleClass();
$var1 = $var1->GetRecord(1);
$var1 = $var2->GetRecord(2);
$var1 = $var3->GetRecord(3);
$var1 = $var4->GetRecord(4);

I know the latter example is more lines of code, but is either example specifically right or wrong? Is there a reason to use one rather than the other?

Thanks in advance,

Chris.

#2

The second example seem to imply something happens with SampleClass when getting a record (as you’re using one instantiation for each get). I assume it’s only a bit wasteful as there is no real reason to use a separate SampleClass instantiation for each get call, but it might be confusing as is.

#3

The former is operating on a single instance of the class, while the latter is is operating on 4 unique instances. So it depends what you are actually trying to do.

#4

Thank you for the responses.

I don’t have any specific reason to instance the class 4 times - I just thought the second way was the easiest way to be honest.

I just couldn’t decide whether there was a real downside to using it the second way. I’m guessing it is primarily a memory overhead issue?

Thanks again,

Chris.

#5

that are two different use cases, you can’t compare them without context.

#6

You’d be surprised how much web servers today can handle. Usually I’m way more concerned about how understandable the code is than how fast it runs. Lots of great talks on youtube on clean code and how important it is as we as devs are really spending more time reading/processing code than we spend writing code. The rather insane amount of bandwidth, cpu, memory and disk io we have available today has kinda made devs lazy. But in fairness hw/bandwidth costs are miniscule compared to dev cost, so it’s kinda to be expected.

#7

So is this a more acceptable use of classes? Is it bad to create a Softback object within a method of the Softback object?

<?php
    spl_autoload_register(function($class) {
        $base_dir = './classes';
        $list_dir = scandir(realpath($base_dir));
        if (isset($list_dir) && !empty($list_dir)) {
            foreach ($list_dir as $list_dir_key => $subDir) {
                $file = $base_dir . DIRECTORY_SEPARATOR . $subDir . DIRECTORY_SEPARATOR . $class . '.php';
                if (file_exists($file)) {
                    require $file;
                }
            }
        }
    });

    $myLibrary = new Library();
    echo "<pre>";
    print_r($myLibrary);
    echo "</pre>";
    die();

class Library
{
    public $book = array();

    public function __construct()
    {
        $item = new Softback;
        $this->book[] = $item->setBook("Bod and Bod!","Janet Smith");
        $this->book[] = $item->setBook("Here comes Bod!","James Jameson");

    }
}

class Softback
{
    public $title = null;
    public $author = null;

    public function setBook($title, $author)
    {
        $result = new Softback();
        $result->title = $title;
        $result->author = $author;
        return $result;
    }
}
#8

I don’t like that approach. There is no reason “Library” should know how to instantiate Softback objects. And the way Softback actually instantiates a new object of itself is kinda confusing.

I’d much rather iterate over a data set outside the Library class in some way like this. I’d also recommend looking into using composer for auto loading classes.

class Library
{
    private $books = [];

    public function addBook(Book $book)
    {
        $this->books[] = $book;
    }
}

class Softback
{
    private $title = null;
    private $author = null;

    public function __construct($title, $author)
    {
        $this->title = $title;
        $this->author = $author;
    }
}

$library = new Library();
$books = $booksModel->getSoftbackBooks();

foreach ($books as $book) {
  $library->addBook($book);
}

Note: for the code above to work you need some sort of books model that can fetch the data. Softback also need to inherit from a parent “book” class so typehinting can be used. I just assumed you’d use multiple kinds of books.

#9

Like @JimL said, Use Composer and PS-4 Autoloading

#10

Thank you for your comments. I’m looking into Composer, and doing so re-coding using what I’ve learnt from you guys - so thank you again.!

JimL - Where you use the following:

public function addBook(Book $book)

which as far as I can see requires $book to be of type Book, if Book had extended classes such as Hardback and Softback is there a way to say it can be either hardback OR softback?

Thanks anyway :slight_smile:

Chris.

#11

Not yet. Do you have specific logic in the library depending on type of book added?

#12

You can go a couple ways with that.

You can either do a Book Abstract class that hard and soft extend. Or you can do an Interface. An Interface would allow you to add magazines, movies, and other things that libraries have and everything that implements the interface is allowable.

Interfaces: https://www.ideone.com/GFILHs

2 Likes