[Phpmyadmin-devel] Refactoring challenge

[Chanaka Dharmarathna]

On Sat, Mar 24, 2012 at 9:59 PM, Alex Marin <alex.ukf at gmail.com> wrote:

>
> On Sat, Mar 24, 2012 at 6:20 PM, Alex Marin <alex.ukf at gmail.com> wrote:
>
>> Hello,
>>
>> I find these refactoring guidelines useful for everyone, so I'll have a
>> go:
>>
>>     $start = 0;
>>     if ( strlen( $type ) > $GLOBALS['cfg']['LimitChars'] ) {
>>         $start = 13; // strlen( '<abbr title="' );
>>         $type = '<abbr title="' . $type . '">'
>>                   . substr( $type, 0, $GLOBALS['cfg']['LimitChars'] )
>>                    . '</abbr>';
>>     }
>>
>>     unset( $field_charset );
>>     $matches_type = ( substr($type, $start, 4)   == 'char'
>>                               || substr($type, $start, 7)   == 'varchar'
>>                               || substr($type, $start, 4)   == 'text'
>>                               || substr($type, $start, 8)   == 'tinytext'
>>                               || substr($type, $start, 10) == 'mediumtext'
>>                               || substr($type, $start, 8)   == 'longtext'
>>                               || substr($type, $start, 3)   == 'set'
>>                               || substr($type, $start, 4)   == 'enum'
>>      );
>>     $field_charset = '';
>>     if ( $matches_type && ! $extracted_fieldspec['binary'] ) {
>>         if ( strpos( $type, ' character set ' ) ) {
>>             $type = substr( $type, 0, strpos( $type, ' character set ' )
>> );
>>         }
>>         if ( ! empty( $row['Collation'] ) ) {
>>             $field_charset = $row['Collation'];
>>         }
>>     }
>>
>>
>> These are some refactoring modifications I would see fit, but I am sure
>> there are others, so I would like to see different approaches.
>>
>> --
>> Alex
>>
>
> Without the 3 unnecessary newlines of course ( that were inserted
> before my copy-pasted sections).
>
> --
> Alex
>
>
> ------------------------------------------------------------------------------
> This SF email is sponsosred by:
> Try Windows Azure free for 90 days Click Here
> http://p.sf.net/sfu/sfd2d-msazure
> _______________________________________________
> Phpmyadmin-devel mailing list
> Phpmyadmin-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>
>
Hi Marc,

I have not learn yet the PHP code refactoring techniques. But I like to
learn them. I tried to do some improvements on that code (tbl_structure.php
: in current master, these start at line 263 of the script).

changes :
* Create an array for store data types ($dataTypesArray)
* Introduced a Boolean to check whether the string manipulating condition
is true or false ($isTypeMatched)
* Since there was a common pattern in conditions, create those condition
check inside a loop using  $isTypeMatched boolean
* Use meaningful non-common names to new variables
* Keep empty spaces between code snippets to increase the readability of
the code

    $start = 0;
    if (strlen($type) > $GLOBALS['cfg']['LimitChars']) {
        $start = 13;
        $type = '<abbr title="' . $type . '">' . substr($type, 0,
$GLOBALS['cfg']['LimitChars']) . '</abbr>';
    }

    unset($field_charset);
    $dataTypesArray = array('char', 'varchar', 'text', 'tinytext',
'mediumtext', 'longtext', 'set', 'enum');
    $isTypeMatched = false;

    foreach ($dataTypesArray as $dataType) {
        if (substr($type, $start, strlen($dataType)) == $dataType) {
            $isTypeMatched = true;
            break;
        }
    }

    if ($isTypeMatched && !$extracted_fieldspec['binary']) {
        if (strpos($type, ' character set ')) {
            $type = substr($type, 0, strpos($type, ' character set '));
        }
        if (!empty($row['Collation'])) {
            $field_charset = $row['Collation'];
        } else {
            $field_charset = '';
        }
    } else {
        $field_charset = '';
    }

With this, though the number of data types need to increase, only needed
change is adding new data types to $dataTypesArray. No need of adding more
conditions.
Does the above mentioned changes are make the code better ?

Regards !
-- 
____________________________________

Chanaka Indrajith
Bsc.Computer Engineering Undergraduate
Faculty of Engineering
University of Peradeniya
____________________________________
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.phpmyadmin.net/pipermail/developers/attachments/20120325/a3459ce1/attachment.html>