Refactoring to MVC in Laravel

I've been working for quite some time to migrate an application from an old, very liniar, non-modular design to the point where it follows the MVC pattern fairly well.

It's a slow process, but it's getting there, here is an example of one of the functions that has been reworked.

The old function:

/**
    * builds a select box, with times as the options
    *
    * @todo should be built to build any selectBox, perhaps with different
    * parameters to decide the type of selectBox, perhaps being used by more
    * specific functions, which all this one.
    *
    * @param string $name
    * @param int $start
    * @param int $end
    * @param int $seconds
    * @param int $recommended
    * @param boolean $selected
    * @return string
    */
    public function selectBox ($name,$start,$end,$seconds,$recommended,$selected = '') {

    $selected = ($selected == '' && $name == 'endtimefrom' ? 3 :
    ($selected == '' && $name == 'endtimeto' ? 7 : $selected));
    $return = "<select name="$name">

"; foreach (range($start, $end) as $hour) { $return .= "<option value='{$hour}'".( $selected == $hour ? ' selected="selected"' : '').">". ($hour == 0 ? 'Midnight' : ($hour == $recommended ? "0{$hour}:00 am (Recommended)" : "0{$hour}:00 am"))." "; } $return .= " "; return $return; }

This function was actually written as the application was being modernised, the old functionality wasn't even in a function, and had a few variations dotted around the application.

As a general rule if you're generating HTML in your backend code, something is wrong!

The new function:

public function selectBox ($name,$start,$end,$seconds,$recommended,$selected = '')
    {
    foreach (range($start, $end) as $hour)
    {
    $values[] = array(
    'value' => $hour,
    'text'  => ($hour == 0 ? 'Midnight' : ($hour == $recommended ?
    "0{$hour}:00 am (Recommended)" :
    "0{$hour}:00 am"))
    );
    }
    $selected = (isset($selected) ?
    $selected : ($name == 'endtimefrom' ? 3 : 7));

    return \View::make(\Utilities\Theme::getViewName("form.selectbox", $this->themeName),
    array (
    'selected'  => $selected,
    'name'      => $name,
    'values'    => $values,
    ));
    }

The new function takes away all of the markup, it could still use a little improvement, as there is some English in there (which should be pulled from the language file, but we have to pick our battles!)

But generally, it's good, it prepares data to be passed to a view in a standardised way, so while it is longer than the original, it allows greater code reuse, and as you might have spotted in the View call, theming.

The view:

<select name="{{ $name }}" id="{{ $name }}" {{
    (!empty($onchange) ? "onchange='$onchange' " : '') }}>
    @foreach ($values as $value)
    @if(!empty($value['value']) && !empty($value['text']))
    <option value='{{ $value['value'] }}'{{ (isset($selected) && $selected == $value ?
    ' selected="selected"' : '') }}>{{ $value['text'] }}</option>
    @endif
    @endforeach
    </select>

As we can see here, the view is pretty concise, with very little logic, beyond adding the 'selected' attribute if the option should be preselected.

Next steps

Will need to have a think!

Any thoughts? Leave a comment!