7

I am creating some reusable objects in php, and I wanted to know whats the best way to build them. Below I have 2 examples of different ways of doing this. 

Class Uploader{
    public $Filename;
    public $Directory;

    function upload(){
        upload_file($this->Filename, $this->Directory)
    }
}

// Then use the class above like this.
$u = new Uploader;
$u->Filename = 'foo.png'; // Set all the props
$u->Directory = 'bar/'    //  ^   ^   ^    ^
$u->upload();             // Then Execute

Or would it be better to do this...

Class Uploader {
    function uploader($filename, $directory){
        upload_file($filename, $directory)
    }
}

// Then use the class above like this.
$u = new Uploader;
$u->uploader('foo.png', 'bar/') // Obviously much less code, All in One.

Out of these two methods, which one is preferred, is their a speed difference or any sort of gain of using one over the other?
I favour example #1, but is their a best practice to this?

  • You certainly use an odd way to define a class. Whats wrong with brackets, like the rest of us use :) – TJHeuvel Sep 01 '11 at 15:24
  • It´s probably just your example, but both seem wrong to me: Wrapping a static function in a function in a class does not seem very useful. If you are on php 5.3+ you can use namespaces for stuff like that. – jeroen Sep 01 '11 at 15:51

4 Answers4

7

Why can't you do both?

class Uploader
{
  public
    $filename,
    $directory;

  public function __construct( $name = '', $dir = '', $autoUpload = false )
  {
    $this->filename = $name;
    $this->directory = $dir;
    if ( $autoUpload )
    {
      $this->upload()
    }
  }

  public function upload()
  {
    //check your values
    ...code...
    upload_file( $this->filename, $this->directory );
  }
}

With this technique, you could automatically upload a file simply with:

$uploader = new Uploader( $name, $dir, true);

or you could manually construct the object with:

$uploader = new Uploader();
$uploader->filename = $name;
$uploader->directory = $dir;
$uploader->upload();
zzzzBov
  • 174,988
  • 54
  • 320
  • 367
  • 1
    @igorw, that's about the most ridiculous thing I've ever read. The constructor is meant to *construct* the object. There is all sorts of work that can and *should* be done in constructors. – zzzzBov Sep 01 '11 at 20:02
  • @zzzzBov there is a pretty extensive article on the subject here: http://misko.hevery.com/code-reviewers-guide/flaw-constructor-does-real-work/ – igorw Sep 01 '11 at 20:10
  • 2
    @igorw, the way it is described in the article is completely irrelevant to the example I posted. The article isn't against having a functional constructor, it's against tight coupling of object instantiation and how they interfere with testing. The summary is: "if it's difficult to test your object constructor, you've made it too complicated". – zzzzBov Sep 01 '11 at 20:44
6

Method one is a classic OO approach where the object you create houses the data and the methods to act upon that data. Method two is simply creating a utility library of functions within a class. Method two is undoubtedly faster, but less OO in its approach. If you are shooting for reuse, I would actually go with method one. If it is performance you want, skip using classes altogether and write a functional library.

Daniel Pereira
  • 1,785
  • 12
  • 10
4

Daniel Pereira is right about performance.

To mix the two examples (perfomance & reuse), you can try: 

Class Uploader{
    public $Filename;
    public $Directory;

    function Uploader($this->Filename, $this->Directory){
       upload_file($this->Filename, $this->Directory);
    }
}
$a = new Uploader('foo.png','bar');
echo $a->Filename; //foo.png
echo $a->Directory; //bar

It should be actually (because of a mistake):

Class Uploader{
        public $Filename;
        public $Directory;

        function Uploader($Filename, $Directory){
            $this->Filename = $Filename;
            $this->Directory = $Directory;
           upload_file($this->Filename, $this->Directory);
        }
    }
Franquis
  • 743
  • 1
  • 5
  • 17
  • Just out of curiosity, can you set protected/private properties like this as well? – kgilden Sep 01 '11 at 15:57
  • Yes you can! But remember that you won't be able to call them with a simple `$a->Filename`... – Franquis Sep 01 '11 at 16:09
  • +1 for the handy tip. No more do I have to use separate variables for assigning the values in my methods! I just wonder if it's not very known or is considered bad practice. – kgilden Sep 01 '11 at 16:13
  • @gilden and Franquis: No you can't. Franquis: What are you talking about?! You must be misunderstanding eachother, and you must have got things mixed up, because `function Uploader($this->Filename, $this->Directory) {` is not valid syntax. I believe that is what @gilden is referring to, and I have the faint idea you, Franquis, are not referring to that. – Decent Dabbler Sep 01 '11 at 22:42
  • You 're actually right @fireeyedboy, I definitly missed two lines of code... It should be `function Uplodaer($filename, $directory)` and two more lines whith `$this->Filename = $filename`,etc My mistake... I just edited my answer... – Franquis Sep 01 '11 at 23:30
2

If you want to do true OO, the first example is pretty good. Another suggestion would be this:

Class Uploader{
    private $Filename;
    private $Directory;

    function upload(){
        upload_file($this->Filename, $this->Directory)
    }

}

Then you can create setFileName and setDirectory methods so you abstract out the setting of those fields for later.

You can also create a constructor with those fields in it. Many ways to solve this problem.

Alan Delimon
  • 817
  • 7
  • 14