HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Select input that is generated and populated with options

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
populatedgeneratedwithoptionsinputthatandselect

Problem

I started to learn PHP OOP recently and searched the web for some practical exercises and I found one that said to build a select input that is generated and populated with options by the object. I just want to ask for some opinions on code quality. Could it be done better? What is the biggest flaw?

The class:

class completeSelect{

    private $setIn;
    private $option;

    public function __construct($val = array()) {
        $this->setIn=$val;

        $this->buildOptions();
    }

    public function buildOptions()
    {
        $this->option = "";
        foreach($this->setIn as $value)
        {
            $this->option .= ''.$value.'';
        }

        return $this->option;
    }

    public function BuildSelect() {
        $bs  = '';
        $bs .= $this->option;
        $bs .= '';

        return $bs;
    }
}


The output:

require_once("class.php");

$n = new completeSelect(array("Mozilla","Opera","Internet Explorer"));
echo $n->buildSelect();

Solution

A few minors:

-
Your spacing is a bit inconsistent (e.g. missing spaces here completeSelect{ and here $this->setIn=$val;)

-
Your naming convention is a bit inconsistent as well. I don't do much PHP but class names are usually PascalCase. You should decide whether your methods should be camelCase or PascalCase (you have buildOptions vs BuildSelect).

-
You have a typo here select_brwoser - should probably be select_browser

-
Your interface into the class is not quite clear. You build your options on construction yet the method to do so is public. Am I supposed to call it again - if so under which circumstances? If the class is immutable (e.g. you can't add more values for the options) then why not simply build the entire select in the first place and cache it?

A possible change would be to make the buildXYZ methods private and build on construction (assuming the class is immutable) and have public getXYZ methods instead which return the cached value for the options or the select respectively.

Context

StackExchange Code Review Q#43427, answer score: 6

Revisions (0)

No revisions yet.