1
\$\begingroup\$

I created this class to handle some dropdowns that I'd been creating using a squirrelly combination of HTML, jQuery, and AJAX. This works much better in the one script that I've deployed it in:

namespace appwpclass;

class PopulateDropdown
{
    public $skillArray;
    public $grpArray;
    public $facArray;
    /**
     * PopulateDropdown constructor.
     */
    public function __construct() {
        $skillArray[] = $this->skillArray;
        $grpArray[] = $this->grpArray;
        $facArray[] = $this->facArray;
    }

    public function skillSelect($category){
        //query to db to get skills
        return $skillArray;
    } //end function

    public function groupSelect($category, $mem_type, $selected_group=NULL){

// query to get list of clients -- it is allowed to be NULL in which case ALL active clients will be returned
        return $grpArray;
    } //end function

    public function facilitySelect($selected_group){
        //query to get list of facilities associated with a $selected_group, which is a required field.
            return $facArray;

    } //end function

} //end class

Now, in my admittedly "procedural" PHP script (which I'm trying to refactor), I have "required" this Class:

require '../appwpclass/PopulateDropdown.php';

And in that script, I have these statements in order to populate the various dropdowns:

$dropdown = new appwpclass\PopulateDropdown();
$skillsArr = $dropdown->skillSelect($category);
$groupArr = $dropdown->groupSelect($category, $memType, $selGroup);
$facilityArr = $dropdown->facilitySelect($selGroup);

And then I use those arrays in filter dropdown HTML, as in this example:

<select name="selGroup" id="selGroup" size="1" class="textfield" onchange="submit()">
 <option value="0">--Any--</option>
<?php
foreach( $groupArr as $group_id=>$group_name ) {
    $selected = ( $group_id == $selGroup ) ? ' selected' : '';
    ?>
    <option value="<?php echo $group_id;?>"<?php echo $selected;?>><?php echo $group_name;?></option>
<?php } ?>

This all works fine in this one script, but I have a lot of scripts that also use one, two, or all of these dropdowns. So I thought about just using them in a "require" somewhere in each script, but I'm thinking there has to be a better OOP way. Maybe I should create another class and instantiate PopulateDropdown in that class's constructor? Or in another method in that other class? And then have a separate method for each of those statements, e.g. a method for

 $skillsArr = $dropdown->skillSelect($category)

, etc.. I guess if I did that, I'd need to "pass" the 3 properties that are required by one or more of these existing methods.

Would anyone please provide me some direction on this? I'm pretty new to OOP and I'm starting to use Laravel but I'm nowhere near ready to refactor this enterprise system yet.

If I used a new class, as I suggested, then obviously I'd require that class or use that namespace in my other scripts. Just seems like that would be more concise and less prone to errors.

\$\endgroup\$
1
  • \$\begingroup\$ When adding additional information you should edit your question instead of adding a comment. I have added that information to your post. Learn more about comments including when to comment and when not to in the Help Center page about Comments. \$\endgroup\$ Commented Aug 21, 2018 at 18:55

1 Answer 1

1
\$\begingroup\$

For such a use case a class would be rather useless. A class is used to encapsulate the stuff that belongs to just a single matter. Here, there is nothing in common between these functions.

If you want to use a class, consider creating a class related to Skills, with one of its methods responsible for returning a list of skills.

but if you want to just refactor your code, a set of functions will do as well. Make your PopulateDropdown.php as

function skillSelect($category){
    //query to db to get skills
    return $skillArray;
} //end function

function groupSelect($category, $mem_type, $selected_group=NULL){

// query to get list of clients
    return $grpArray;
} //end function

function facilitySelect($selected_group){
    //query to get list of facilities associated with a $selected_group
        return $facArray;

} //end function

and then

require '../appwpclass/PopulateDropdown.php';
$skillsArr = skillSelect($category);
$groupArr = groupSelect($category, $memType, $selGroup);
$facilityArr = facilitySelect($selGroup);

So you will have the desired functionality without a single class.

You may also want to create another helper function, to reduce the amount of duplicated code,

function show_option($array, $selected) {
    foreach( $array as $id => $name ) {
        $selected = ( $id == $selected ) ? ' selected' : '';
        ?>
        <option value="<?= $id ?>"<?= $selected ?>><?= $name ?></option>
        <?php 
    } 
}

which will make your HTML much cleaner

<select name="selGroup" id="selGroup" size="1" class="textfield" onchange="submit()">
    <option value="0">--Any--</option>
    <?php show_option($groupArr, $selGroup); ?>

So I thought about just using them in a "require" somewhere in each script, but I'm thinking there has to be a better OOP way

Well, to use some function or a class you must require them, this way or another. Although classes could be autoloaded, you have to require the autoloader definition anyway. So, although there are other ways, your current approach is good enough.

\$\endgroup\$
2
  • \$\begingroup\$ Much thanks for code in the show_option function. For some reason though, the only option it would maintain in the dropdown list when selected was the very first one (after the '--Any--' option). If I selected the 3rd or the 5th or the 10th or whatever, the dropdown list always reverted back to '--Any--'. This was true on all four separate dropdowns. I solved it by changing the 'selected' parameter to 'selected_id' & then modifying the code thusly: $selected = ( $id == $selected_id ) ? ' selected' : ''; . Now, whatever option I select is maintained in the dropdown as the ' selected' one. \$\endgroup\$
    – McAuley
    Commented Aug 22, 2018 at 23:26
  • \$\begingroup\$ It's just impossible, as the parameter in the function definition is called $selected, and there is no other variable called $selected_id, so it was probably some mistake on your part. But I am glad you resolved it, this way or another \$\endgroup\$ Commented Aug 23, 2018 at 8:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.