Please login or register.

Login with username, password and session length
Advanced search  

News:

You need/want an older version of sNews ? Download an older/unsupported version here.

Pages: [1] 2 3 ... 9

Author Topic: Improve one function a week  (Read 21359 times)

skian

  • Full Member
  • ***
  • Karma: 14
  • Posts: 120
Improve one function a week
« on: April 18, 2009, 02:39:28 PM »

Hi guys,
Every time I have a look at Snews code I notice overly complex code, redondancy, pooly testable function etc...
So i'm starting this thread where I'll post small imporvements. I hope others will comtribute too...
Logged

skian

  • Full Member
  • ***
  • Karma: 14
  • Posts: 120
Re: Improve one function a week
« Reply #1 on: April 18, 2009, 02:40:19 PM »

replacement for delete_cat
Code: [Select]
function delete_cat($id){
        $catdata = mysql_fetch_array(mysql_query("SELECT catorder,subcat FROM "._PRE.'categories'." WHERE id = $id"));
        $cat_order = $catdata['catorder'];
        $cat_subcat = $catdata['subcat'];
        mysql_query("DELETE FROM "._PRE.'categories'." WHERE id = $id LIMIT 1");
        mysql_query("UPDATE "._PRE.'categories'." SET
                catorder = catorder - 1 WHERE catorder > $cat_order AND subcat = $cat_subcat");
}

What this change is about : reduce the number of sql requests (1 request to update all catorder instead of N+1)
« Last Edit: April 18, 2009, 11:11:45 PM by skian »
Logged

skian

  • Full Member
  • ***
  • Karma: 14
  • Posts: 120
Re: Improve one function a week
« Reply #2 on: April 18, 2009, 02:54:14 PM »

replacement for categories & subcategores
Code: [Select]

// DISPLAY CATEGORIES
function categories() {
        global $categorySEF;
        $tab = retr_categories();
        if (sizeof($tab) > 0) {
                foreach($tab as $r) {
                        $category_title = $r['seftitle'];
                        $r['name'] = (s('language')!='EN' && $r['name'] == 'Uncategorized' && $r['parent']==1) ? l('uncategorised')
: $r['name'];
                        $class = $category_title == $categorySEF ? ' class="current"' : '';
                        if (isset($r['total'])) { $num='('.$r['total'].')'; }
                        echo '<li><a'.$class.' href="'._SITE.$category_title.'/" title="'.$r['name'].' - '.$r['description'].'">'.$r
['name'].$num.'</a>';
                        $parent = $r['parent'];
                        if ($category_title == $categorySEF) { subcategories($parent); }
                        echo '</li>';
                }
        } else {
                echo '<li>'.l('no_categories').'</li>';
        }
}

function subcategories($parent) {
        global $categorySEF, $subcatSEF;
        $tab = retr_categories($parent);
        if (sizeof($tab) > 0) {
                echo '<ul>';
                foreach($tab as $s) {
                        $subSEF = $s['seftitle'];
                        $class = $subSEF == $subcatSEF ? ' class="current"' : '';
                        if (isset($s['total'])) { $num=' ('.$s['total'].')'; }
                        echo '<li class="subcat">
                                <a'.$class.' href="'._SITE.$categorySEF.'/'.$subSEF.'/" title="'.$s['description'].'">
                                '.$s['name'].$num.'</a></li>';
                }
                echo '</ul>';
        }
}

// RETRIEVE CATEGORIES OR SUBCATEGORIES FROM DB
function retr_categories($parent = 0) {
        $qwr = !_ADMIN ? ' AND a.visible=\'YES\'' : '';
        if (s('num_categories') == 'on') {
                $count = ', COUNT(DISTINCT a.id) as total';
                $join = 'LEFT OUTER JOIN '._PRE.'articles'.' AS a
                        ON (a.category = c.id AND a.position = 1  AND a.published = 1'.$qwr.')';
        } else {
                $count ='';
                $join='';
        }
        $result = mysql_query('SELECT
                        c.seftitle, c.name, description, c.id AS parent'.$count.'
                FROM '._PRE.'categories'.' AS c '.$join.'
                WHERE c.subcat = '.$parent.' AND c.published = \'YES\'
                GROUP BY c.id
                ORDER BY c.catorder,c.id');
        $tab = Array();
        while ($r = mysql_fetch_array($result)) {
                $tab[] = $r;
        }
        return $tab;
}


What the change is about :
- factorize SQL request for cat or subcat
- function retr_catagories is more testable because of separation of query (SQL) and printing (HTML)
« Last Edit: April 18, 2009, 11:14:19 PM by skian »
Logged

centered

  • Guest
Re: Improve one function a week
« Reply #3 on: April 18, 2009, 03:52:52 PM »

replacement for delete_cat
Code: [Select]
function delete_cat($id){
        $catdata = mysql_fetch_array(mysql_query("SELECT catorder,subcat FROM "._PRE.'categories'." WHERE id = $id"));
        $cat_order = $catdata['catorder'];
        $cat_subcat = $catdata['subcat'];
        mysql_query("DELETE FROM "._PRE.'categories'." WHERE id = $id LIMIT 1");
        mysql_query("UPDATE "._PRE.'categories'." SET
                catorder = catorder - 1 WHERE catorder > $cat_order AND subcat = $cat_subcat");
}

You should unset $catdata, $cat_order, and $cat_subcat to free memory;
Logged

skian

  • Full Member
  • ***
  • Karma: 14
  • Posts: 120
Re: Improve one function a week
« Reply #4 on: April 18, 2009, 04:09:22 PM »

You should unset $catdata, $cat_order, and $cat_subcat to free memory;
hmm, no : the scope of these vars is the function. these vars are freed when the function exists.
Logged

centered

  • Guest
Re: Improve one function a week
« Reply #5 on: April 18, 2009, 04:20:07 PM »

Thanks for pointing that out, I forgot.
Logged

Patric Ahlqvist

  • Nobodys perfect, but Im pretty effing close
  • ULTIMATE member
  • ******
  • Karma: 65
  • Posts: 4867
  • “I'm a self-made man and worships my creator.”
    • p-ahlqvist.com
Re: Improve one function a week
« Reply #6 on: April 18, 2009, 05:55:17 PM »

Could you also post why you do these replecements with the replecement itself, so one would understand why you did what you think is an imorivment. That would help, I think, people to understand what you do and why...

Just a note from moi ;)
Logged
"It's only dead fish that goes with the flow... "
Updated

Sven

  • ULTIMATE member
  • ******
  • Karma: 88
  • Posts: 2029
  • Chasing MY bugs!
    • hiseo.fr - rédacteur Web
Re: Improve one function a week
« Reply #7 on: April 18, 2009, 06:31:10 PM »

Joost

  • Guest
Re: Improve one function a week
« Reply #8 on: April 18, 2009, 06:35:47 PM »

Could you also post why you do these replecements with the replecement itself, so one would understand why you did what you think is an imorivment. That would help, I think, people to understand what you do and why...

Just a note from moi ;)

What Skian wants, is basically a cleanup (links to restricted part of the forum, Dides only):
- Removal of obsolete and redundant code.
- Refactoring code (breaking large functions down into smaller code chunks/functions)

Doing this should have no effect on user level. So no explaining is needed on user level.
Doing this, should result in improvement of:
- Maintainability
- bug tracking
- bug resolving
- mod/system development





Logged

Patric Ahlqvist

  • Nobodys perfect, but Im pretty effing close
  • ULTIMATE member
  • ******
  • Karma: 65
  • Posts: 4867
  • “I'm a self-made man and worships my creator.”
    • p-ahlqvist.com
Re: Improve one function a week
« Reply #9 on: April 18, 2009, 06:53:32 PM »

Yupp, I reconed that, Joost. Just thought it might be good to tell whomever applying the updated code what he done and why... Although the first reason for him to post IS all of what you mentioned. I'm no resistance here... Feel it's great if code that needs to be improved will be just that.

"So no explaining is needed on user level" Au contrare, I think. If an easy explaination was provided perhaps someone would learn something.
Logged
"It's only dead fish that goes with the flow... "
Updated

skian

  • Full Member
  • ***
  • Karma: 14
  • Posts: 120
Re: Improve one function a week
« Reply #10 on: April 18, 2009, 11:09:20 PM »

in admin.js : replacement for 2 last string replacement

Code: [Select]
str = str.replace(/[\s.-]+/g, "-");
str = str.replace(/([^a-z 0-9-]+|[-]$)/g,'');

What does the replacement : better handling of special char (ponctuation, spaces at end of title, etc...)

Edt : even better code
Code: [Select]
str = str.replace(/([\W]+$)/g,'');
str = str.replace(/[\W]+/g, "-");
« Last Edit: April 26, 2009, 07:51:35 PM by skian »
Logged

skian

  • Full Member
  • ***
  • Karma: 14
  • Posts: 120
Re: Improve one function a week
« Reply #11 on: April 18, 2009, 11:15:34 PM »

"So no explaining is needed on user level" Au contrare, I think. If an easy explaination was provided perhaps someone would learn something.

D'accord - I'll try to explain the code change (initial posts updated)
Logged

Joost

  • Guest
Re: Improve one function a week
« Reply #12 on: April 19, 2009, 04:13:18 AM »


Yupp, I reconed that, Joost. Just thought it might be good to tell whomever applying the updated code what he done and why... Although the first reason for him to post IS all of what you mentioned. I'm no resistance here... Feel it's great if code that needs to be improved will be just that.

"So no explaining is needed on user level" Au contrare, I think. If an easy explaination was provided perhaps someone would learn something.

I know it sounds cocky. However, these are not modifications that add/change functionality or improve performance, noticeable to the user.
Improvements as described are only useful to developers and only if they are part of an official distribution.
Thus, there is the follow-up question: "Should they be part of the next version?" 
My answer would be: Non, Monsieur.
1.7 mods needs to remain compatible. The community needs/deserves stability.




Logged

skian

  • Full Member
  • ***
  • Karma: 14
  • Posts: 120
Re: Improve one function a week
« Reply #13 on: April 19, 2009, 02:44:03 PM »

1.7 mods needs to remain compatible. The community needs/deserves stability.

I don't know whether these changes should be applied in 1.7 or 1.7.1 or 1.8. Let the dudes decide
I think that what the community really needs is clean code : easier to maintain, easier to understand, easier to mod
Logged

skian

  • Full Member
  • ***
  • Karma: 14
  • Posts: 120
Re: Improve one function a week
« Reply #14 on: April 19, 2009, 03:54:01 PM »

Create a function to delete category (and all content : subcats, articles & comments)
Code: [Select]
function delete_category_all($id) {
        // delete comments from articles in cat
        mysql_query("DELETE comments.* from comments
                LEFT JOIN articles ON articles.id = comments.articleid
                WHERE articles.category = $id ");
        // delete comments from articles in subcat
        mysql_query("DELETE comments.* from comments
                LEFT JOIN articles ON articles.id = comments.articleid
                LEFT JOIN categories subcat ON articles.category = subcat.id
                WHERE categories.subcat = $id ");
        // delete articles in category
        mysql_query("DELETE FROM "._PRE.'articles'." WHERE category = $id");
        // delete articles in subcat
        mysql_query("DELETE articles.* FROM articles
                LEFT JOIN categories subcat ON articles.category = subcat.id
                WHERE categories.subcat = $id ");
        // delete subcats
        mysql_query("DELETE FROM "._PRE.'categories'." WHERE subcat = $id");
        // delete category
        delete_cat($id);
}


Update function processing accordingly

Code: [Select]
                case 'delete_category_all':
                        delete_category_all($id);
                        echo notification(0,'', 'snews_categories');
                        break;
Logged
Pages: [1] 2 3 ... 9