Code Review help: too complicated or not?

1

I have a form.ctp with usual fields:

        echo $this->Form->input('dob');
        echo $this->Form->input('age');

I’ve asked a developer to finish the form, so I was expecting this:

        echo $this->Form->input('dob', ['type' =>...., 'label'=> ...]);

Instead, they have created a custom FormHelper overwriting some Helper/FormHelper functions and having an init Schema, as well as adding Traits, changes to entity, aftersave:

// in form.ctp now:
        echo $this->CustomForm->input('dob');
        echo $this->CustomForm->input('age');


//in CustomFormHelper
...
protected function initFields(){
        $this->fields = [
            ...
            'dob' => [
                'label' => ['text' => '5. Date of birth'],
                'type' => 'text',
            ],
            'age' => [
                'label' => ['text' => '6. Age'],
            ],
...

//in the FormEntity

..    protected function _setDob($data)
    {
        return $this->DateFormatSQL($data);
    }

    protected function _getDob($data) 
    {
        if($this->needDecoding()){
            return $this->DateFormatHTML($data);
        }
        return $data;
    }

It does seem clever, but it doesn’t follow convention, seems overkill? Also why not just add the field details per field, why have an init schema and use a custom helper? And there’s all this extra stuff in the Entity and Table. I do agree that CakePHP default date selects, needed to be changed to a datepicker/text field, but surely there’s a simpler way to do it.

2

I think you’re right. But I’m able to use goto in PHP so I may be not proper advisor :wink:

3

Definately not a good way of doing this and is way way over complicating it.

What does DateFormatSQL() do? If it does what I think, then the developer doesn’t seem to understand the CakePHP ORM at all (Which handles things like formatting dates into SQL compatible formats for you).

You should also definately not create a custom helper just for a single form. You can easily replace the entire custom helper with a simple $this->Form->inputs($fields) where $fields is the same array as you are showing in the helper initFields() method.

1 Like
4

Thanks guys. Regards the date function, users want to type dates DD/MM/YYYY. Cake bake creates select options, so we have to turn them into short text input fields (with a jquery date picker). And then I believe turn them YYYY-MM-DD for going into an SQL database. @dakota are you saying that we can leave the date as DD/MM/YYYY and Cake will handle it?

DateFormatSQL is a custom function they wrote:

    public function DateFormatSQL($date = null){
        if(!empty($date)){
            if(preg_match('#/#m', $date)){
                $array = explode('/', $date);
                if(isset($array[0]) && isset($array[1]) && isset($array[2])){
                    switch (self::HTML_DATE_FORMAT){
                        case 'mm/dd/yyyy':
                            $date = $array[1].'-'.$array[0].'-'.$array[2];
                            break;
                        case 'yyyy/dd/mm':
                            $date = $array[1].'-'.$array[2].'-'.$array[0];
                            break;
                        case 'dd/yyyy/mm':
                            $date = $array[0].'-'.$array[2].'-'.$array[1];
                            break;
                        case 'mm/yyyy/dd':
                            $date = $array[2].'-'.$array[0].'-'.$array[1];
                            break;
                       default:
                            $date = $array[0].'-'.$array[1].'-'.$array[2];
                    };
                }
            }
            $date = preg_replace('/Z$/im', 'UTC', $date);
            
            $date = Time::parse($date);
        }
        return $date;
    }
5

CakePHP should definately convert it.

6

just set in bootstrap your locale and cake handle the conversion to sql <-> print

https://book.cakephp.org/3.0/en/core-libraries/internationalization-and-localization.html#parsing-localized-datetime-data

1 Like