[Phpmyadmin-devel] Refactoring challenge

[Alex Marin]

On Sat, Mar 24, 2012 at 3:24 PM, Marc Delisle <marc at infomarc.info> wrote:

> Hi,
>
> As refactoring ideas are numerous in the GSoC 2012 ideas list, I propose
> a challenge. The goal is to discuss on this list an existing code
> snippet from phpMyAdmin and to arrive at the best possible refactored
> version.
>
> At the same time, potential students can show their refactoring skills
> or learn what refactoring is about.
>
> Let's discuss these lines from tbl_structure.php (in current master,
> these start at line 263 of the script):
>
>    $start = 0;
>    if (strlen($type) > $GLOBALS['cfg']['LimitChars']) {
>        $start = 13;
>        $type = '<abbr title="' . $type . '">' . substr($type, 0,
> $GLOBALS['cfg']['LimitChars']) . '</abbr>';
>    }
>
>    unset($field_charset);
>    if ((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')
>        && !$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 = '';
>    }
>

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.phpmyadmin.net/pipermail/developers/attachments/20120324/abd1f200/attachment.html>