31

I'm designing a class that defines a highly complex object with a ton (50+) of mostly optional parameters, many of which would have defaults (eg: $type = 'foo'; $width = '300'; $interactive = false;). I'm trying to determine the best way to set up the constructor and instance/class variables in order to be able to:

  • make it easy to use the class
  • make it easy to auto-document the class (ie: using phpDocumentor)
  • code this elegantly

In light of the above, I don't want to be passing the constructor a ton of arguments. I will be passing it a single hash which contains the initialization values, eg: $foo = new Foo(array('type'=>'bar', 'width'=>300, 'interactive'=>false));

In terms of coding the class, I still feel like I would rather have...

class Foo {
    private $_type = 'default_type';
    private $_width = 100;
    private $_interactive = true;

    ...
}

...because I believe this would facilitate documentation generation (you get the list of the class' properties, which lets the API user know what 'options' they have to work with), and it "feels" like the right way to do it.

But then you run into the problem of mapping the incoming parameters in the constructor to the class variables, and without exploiting the symbol table, you get into a "brute force" approach which to me defeats the purpose (though I'm open to other opinions). E.g.:

function __construct($args){
    if(isset($args['type'])) $_type = $args['type']; // yuck!
}

I've considered creating a single class variable that is itself an associative array. Initializing this would be really easy then, e.g.:

private $_instance_params = array(
    'type' => 'default_type',
    'width' => 100,
    'interactive' => true
);

function __construct($args){
    foreach($args as $key=>$value){
        $_instance_params[$key] = $value;
    }
}

But this seems like I'm not taking advantage of native features like private class variables, and it feels like documentation generation will not work with this approach.

Thanks for reading this far; I'm probably asking a lot here, but I'm new to PHP and am really just looking for the idiomatic / elegant way of doing this. What are your best practices?


Addendum (details about this particular Class)

It's quite likely that this class is trying to do too much, but it is a port of an old Perl library for creating and processing forms. There's probably a way of dividing the configuration options to take advantage of inheritance and polymorphism, but it may actually be counter-productive.

By request, here is a partial listing of some of the parameters (Perl code). You should see that these don't map very well to sub-classes.

The class certainly has getters and setters for many of these properties so the user can over-ride them; the objective of this post (and something the original code does nicely) is to provide a compact way of instantiating these Form objects with the required parameters already set. It actually makes for very readable code.

# Form Behaviour Parameters
        # --------------------------
        $self->{id}; # the id and the name of the <form> tag
        $self->{name} = "webform"; # legacy - replaced by {id}
        $self->{user_id} = $global->{user_id}; # used to make sure that all links have the user id encoded in them. Usually this gets returned as the {'i'} user input parameter
        $self->{no_form}; # if set, the <form> tag will be omitted
        $self->{readonly}; # if set, the entire form will be read-only
        $self->{autosave} = ''; # when set to true, un-focusing a field causes the field data to be saved immediately
        $self->{scrubbed}; # if set to "true" or non-null, places a "changed" radio button on far right of row-per-record forms that indicates that a record has been edited. Used to allow users to edit multiple records at the same time and save the results all at once. Very cool.
        $self->{add_rowid}; # if set, each row in a form will have a hidden "rowid" input field with the row_id of that record (used primarily for scrubbable records). If the 'scrubbed' parameter is set, this parameter is also automatically set. Note that for this to work, the SELECT statement must pull out a unique row id. 
        $self->{row_id_prefix} = "row_"; # each row gets a unique id of the form id="row_##" where ## corresponds to the record's rowid. In the case of multiple forms, if we need to identify a specific row, we can change the "row_" prefix to something unique. By default it's "row_"

        $self->{validate_form}; # parses user_input and validates required fields and the like on a form
        $self->{target}; # adds a target window to the form tag if specified
        $self->{focus_on_field}; # if supplied, this will add a <script> tag at the end of the form that will set the focus on the named field once the form loads.
        $self->{on_submit}; # adds the onSubmit event handler to the form tag if supplied
        $self->{ctrl_s_button_name}; # if supplied with the name of the savebutton, this will add an onKeypress handler to process CTRL-S as a way of saving the form

        # Form Paging Parameters
        # ----------------------
        $self->{max_rows_per_page}; # when displaying a complete form using printForm() method, determines the number of rows shown on screen at a time. If this is blank or undef, then all rows in the query are shown and no header/footer is produced.
        $self->{max_pages_in_nav} = 7; # when displaying the navbar above and below list forms, determines how many page links are shown. Should be an odd number
        $self->{current_offset}; # the current page that we're displaying
        $self->{total_records}; # the number of records returned by the query
        $self->{hide_max_rows_selector} = ""; # hide the <select> tag allowing users to choose the max_rows_per_page
        $self->{force_selected_row} = ""; # if this is set, calls to showPage() will also clear the rowid hidden field on the form, forcing the first record to be displayed if none were selected
        $self->{paging_style} = "normal"; # Options: "compact"

We can, of course, allow ourselves to be drawn into a more lengthy debate around programming style. But I'm hoping to avoid it, for the sanity of all involved! Here (Perl code, again) is an example of instantiating this object with a pretty hefty set of parameters.

my $form = new Valz::Webform (
            id                      => "dbForm",
            form_name               => "user_mailbox_recip_list_students",
            user_input              => \%params,
            user_id                 => $params{i},
            no_form                 => "no_form",
            selectable              => "checkbox",
            selectable_row_prefix   => "student",
            selected_row            => join (",", getRecipientIDsByType('student')),
            this_page               => $params{c},
            paging_style            => "compact",
            hide_max_rows_selector  => 'true',
            max_pages_in_nav        => 5
        );
Tom Auger
  • 19,421
  • 22
  • 81
  • 104
  • 3
    This sounds like the class is doing waaaaaaaaay to much. Can you elaborate some more on what this class is supposed to do and maybe list some more or all of the 50 properties. – Gordon May 11 '11 at 16:14
  • What's the drawback of making these public members? Do you need them to be fixed after construction and do not provide any other means to change the values? – Mel May 11 '11 at 17:00
  • @Mel - unless I'm mistaken, making the members public doesn't improve the situation. It would just encourage some ugly code on the front-end rather than the back-end. – Tom Auger May 11 '11 at 17:17
  • 2
    Since you didnt update your question to include the requested information, I'll only link you to the [Large Class Code Smell](http://sourcemaking.com/refactoring/large-class). I encourage you to read it and apply the suggested refactorings. – Gordon May 11 '11 at 17:24
  • 1
    @Gordon - easy there buddy, give a guy time! You're probably right about the Large Class syndrome here - I hope my explanations in the addendum address (if not justify) that concern. In particular, Class Extraction or Subclass Extraction doesn't facilitate the kind of "short cut" instantiation I would like to make available to the user of the API. Hope this makes sense. Thanks for the link though, I had forgotten about that awesome site. – Tom Auger May 11 '11 at 17:54
  • 1
    @Tom thanks for the update. The class is definitely doing too much. This is easy to spot by the division of properties in Form Behavior and Form Paging. Though it's not a "shortcut", I'd still try to refactor it into smaller parts to take out the complexity of the various components. For instantiating, you could use use a Factory or Builder pattern. – Gordon May 11 '11 at 18:15
  • @Gordon - thanks for the feedback. I'll definitely take that advice into consideration. For the moment I'm looking for a more-or-less verbatim port before I go in and refactor or redesign the class structure. – Tom Auger May 11 '11 at 19:41

6 Answers6

10

I can think of two ways of doing that. If you want to keep your instance variables you can just iterate through the array passed to the constructor and set the instance variable dynamically:

    <?php

    class Foo {
        private $_type = 'default_type';
        private $_width = 100;
        private $_interactive = true;

        function __construct($args){
            foreach($args as $key => $val) {
                $name = '_' . $key;
                if(isset($this->{$name})) {
                    $this->{$name} = $val;
                }
            }
        }
    }

    ?>

When using the array approach you don't really have to abandon documentation. Just use the @property annotations in the class body:

<?php

/**
 * @property string $type
 * @property integer $width
 * @property boolean $interactive
 */
class Foo {
    private $_instance_params = array(
        'type' => 'default_type',
        'width' => 100,
        'interactive' => true
    );

    function __construct($args){
        $this->_instance_params = array_merge_recursive($this->_instance_params, $args);
    }

    public function __get($name)
    {
        return $this->_instance_params[$name];
    }

    public function __set($name, $value)
    {
        $this->_instance_params[$name] = $value;
    }
}

?>

That said, a class with 50 member variables is either only used for configuration (which can be split up) or it is just doing too much and you might want to think about refactoring it.

Daff
  • 43,734
  • 9
  • 106
  • 120
  • I like both of your approaches, and hadn't really thought through that I would be accessing the member variables using $this and so could programatically access them. Does isset() return true if the variable is declared but not assigned a value (I'm not even sure if that makes sense in PHP). I'm just thinking - what if I don't have a default for a certain value (e.g.: `private $_foo;`)? – Tom Auger May 11 '11 at 17:25
  • I would assign null to these. That way you can check for if(isset($this->{$name}) || $this->{$name} === null)... – Daff May 11 '11 at 17:41
  • Just to be clear then, if I were to implement your method 1 above, I _must_ assign some kind of value when declaring the class properties? ie: `private $foo = null;` and must avoid `private $foo;`? I thought isset() returned false if the value were null, and I thought declaring a member variable without assigning it a value assigned it null? – Tom Auger May 11 '11 at 19:45
  • 3
    `private $foo = null;` and `private $foo;` is exactly the same and isset will return false. Use [property_exists](http://fi2.php.net/property_exists) if you want to check if you defined the property in your class at all. – Daff May 11 '11 at 20:31
  • Thanks for all the great info Daff – Tom Auger May 12 '11 at 14:34
  • I realise this is very old, but for others reading you can use `property_exists` to check for existent but unassigned and `null` properties. – fubar Nov 28 '17 at 22:43
7

Another approach is to instantiate the class with a FooOptions object, acting solely as an options container:

<?php
class Foo 
{
    /*
     * @var FooOptions
     */
    private $_options;

    public function __construct(FooOptions $options) 
    {
        $this->_options = $options;
    }
}


class FooOptions
{
    private $_type = 'default_type';
    private $_width = 100;
    private $_interactive = true;

    public function setType($type);
    public function getType();

    public function setWidth($width);
    public function getWidth();

    // ...
}

Your options are well documented and you have an easy way to set/retrieve them. This even facilitates your testing, as you can create and set different options objects.

I don't remember the exact name of this pattern, but I think it's Builder or Option pattern.

Luiz Damim
  • 3,803
  • 2
  • 27
  • 31
  • This actually sounds more like a Model pattern. I'm not sure your suggestion actually works well unless you define an interface (can PHP do that?) IFooOptions and then have the API user implement that interface in a MyOptions class, or extend that class (eg: MyFooOptions extends FooOptions) and pass an instance of that to the Foo constructor. This could work if the user is only instantiating Foo a handful of times. In situations where the user is creating many many instances of this class, and the parameters may need to be dynamically set, it becomes very awkward. – Tom Auger May 11 '11 at 17:48
  • Yeah, PHP can define interfaces in the same way as classes: `interface IFooInterface`. Surely this method has its weaknesses and makes users life harder, but you get a well-defined API with your class options. Maybe for your case this is not the best option... :) – Luiz Damim May 11 '11 at 19:26
3

Just to follow up with how I implemented this, based on one of Daff's solutions:

    function __construct($args = array()){
        // build all args into their corresponding class properties
        foreach($args as $key => $val) {                
            // only accept keys that have explicitly been defined as class member variables
            if(property_exists($this, $key)) {
                $this->{$key} = $val;
            }
        }
    }

Improvement suggestions welcomed!

Community
  • 1
  • 1
Tom Auger
  • 19,421
  • 22
  • 81
  • 104
0

I use this on a few of my classes. Makes it easy to copy and paste for rapid development.

private $CCNumber, $ExpMonth, $ExpYear, $CV3, $CardType;
function __construct($CCNumber, $ExpMonth, $ExpYear, $CV3, $CardType){
    $varsValues = array($CCNumber, $ExpMonth, $ExpYear, $CV3, $CardType);
    $varNames = array('CCNumber', 'ExpMonth', 'ExpYear', 'CV3', 'CardType');
    $varCombined = array_combine($varNames, $varsValues);
    foreach ($varCombined as $varName => $varValue) {$this->$varName = $varValue;}
}

Steps to use:

  1. Paste in and get the list of variables from your current __construct function, removing any optional parameter values
  2. If you haven't already, paste that in to declare your variables for your class, using the scope of your choosing
  3. Paste that same line into the $varValues and $varNames lines.
  4. Do a text replace on ", $" for "', '". That'll get all but the first and last that you'll have to manually change
  5. Enjoy!
Beachhouse
  • 4,972
  • 3
  • 25
  • 39
0

Just a little improvement on Daff's first solution to support object properties that may have a null default value and would return FALSE to the isset() condition:

<?php

class Foo {
    private $_type = 'default_type';
    private $_width = 100;
    private $_interactive = true;
    private $_nullable_par = null;

    function __construct($args){
        foreach($args as $key => $val) {
            $name = '_' . $key;
            if(property_exists(get_called_class(),$name))
                $this->{$name} = $val;
            }
        }
    }
}

?>
bytepan
  • 361
  • 2
  • 5
0

You also could make a parent class.

In that class you only define the variables.

protected function _SetVarName( $arg ){

   $this->varName=$arg;
}

Then extend that class into a new file and in that file you create all your processes.

So you get

classname.vars.php
classname.php

classname extends classnameVars {

}

Because most will be on default you only have to Set/Reset the ones you need.

$cn=new classname();
$cn->setVar($arg);    
//do your functions..
JohnP
  • 49,507
  • 13
  • 108
  • 140
Paolo_Mulder
  • 1,233
  • 1
  • 16
  • 28
  • If I were going to force the API user to do that, why not just use public variables in the class and let the user set them directly? – Tom Auger May 11 '11 at 17:30