On Sat, Mar 24, 2012 at 3:24 PM, Marc Delisle <marc@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