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 !
--
____________________________________

Chanaka Indrajith
Bsc.Computer Engineering Undergraduate
Faculty of Engineering
University of Peradeniya
____________________________________