Re: [Phpmyadmin-devel] Refactoring challenge

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

On Sat, Mar 24, 2012 at 6:20 PM, Alex Marin alex.ukf@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@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 !