Please login or register.

Login with username, password and session length
Advanced search  
Pages: 1 2 [3] 4 5 ... 9

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

Fred K

  • Still trying to learn stuff
  • ULTIMATE member
  • ******
  • Karma: 130
  • Posts: 2728
    • Personal
Re: Improve one function a week
« Reply #30 on: October 08, 2009, 11:40:29 PM »

Fred - I thought I fixed that here: http://snewscms.com/forum/index.php?topic=7996.0 I think that would solve that problem and some other problems noted frequently, but some people need to realize the current functionality hinders usability as you noted.

Jason, you may well have fixed it, but --as far as I know-- that fix isn't in the official download package? That's what I meant by "making it into the core". At least, in the version I downloaded last (around mid-july), your fix wasn't included. So, I'd have to remember your fix and incorporate it (or simply move the archive myself, which I do). Which shouldn't be needed, imho. Unless of course I'm the only one who hates, erm, dislikes having user-made pages pop up in between system pages. ;)

Every time I do a fresh round of templates I start by downloading the official package to make sure I have the most recent version of unmodded files, since I build my templates without mods (most of 'em anyway). So "fixes-for-everyone" should make it into the official files regularly. I know we're short-handed and volonteering, but anyway.

Ok, I've lost track of where I was going with this. I need some coffee.
Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Improve one function a week
« Reply #31 on: October 09, 2009, 01:22:55 AM »

Simple code refactoring
Code: [Select]
<?php
// DISPLAY PAGES
function pages() {
function pages_li($url$text) {
global $categorySEF;
$class = ($categorySEF == $url) ? ' class="current"''';
if (!empty($url)) $url .= '/';
echo '<li><a'.$class.' href="'._SITE.$url.'">'.$text.'</a></li>';
}
pages_li(''l('home'));
pages_li('archive'l('archive'));
$qwr = !_ADMIN ' AND visible=\'YES\'' '';
$query "SELECT id, seftitle, title FROM "._PRE.'articles'." WHERE position = 3 $qwr ORDER BY artorder ASC, id";
$result mysql_query($query);
while ($r mysql_fetch_array($result)) {
if ($r['id'] != s('display_page')) {
pages_li($r['seftitle'], $r['title']);
}
}
pages_li('contact'l('contact'));
pages_li('sitemap'l('sitemap'));
}
?>

Ok I think this one can be done better based on skian's idea (I like it.... )

Code: [Select]
<?php

function buildLIst$str$array ) {
if ( !$array || !is_array$array ) ) return;
foreach ( $array as $key=>$value ) {
$currClass = ( $str == $value['seftitle'] )
' class="current"' 
'';
echo '
<li>
<a' 
$currClass ' href="' _SITE $value['seftitle'] . '">' $value['title'] . '</a>
</li>
'
;
}


// DISPLAY PAGES
function pages() {
global $categorySEF,$_No3;
$home = array(
array( "seftitle" => """title" => l('home') )
);
$archive = array(
array( "seftitle" => "archive""title" => l('archive') )
);
$vis = !_ADMIN 
' AND visible=\'YES\'' 
'';
$query "SELECT id, seftitle, title FROM "._PRE.'articles'.
WHERE position = 3 
$vis ORDER BY artorder ASC, id";
$result mysql_query($query);
$num mysql_num_rows($result);
while ( $r mysql_fetch_assoc$result ) ) { 
if ( $r['id'] != s'display_page' ) ) {
unset( $r['id'] );
$pageResult[] = $r
}
}
$contact = array(
array( "seftitle" => "contact""title" => l('contact') )
);
$sitemap = array(
array( "seftitle" => "sitemap""title" => l('sitemap') )
);
$pageArray array_merge$home$archive$pageResult$contact$sitemap );
buildLIst$categorySEF$pageArray );
if ($num$_No3 true;
}

/*  Original Function
function pages() {
global $categorySEF,$_No3;
$qwr = !_ADMIN ? ' AND visible=\'YES\'' : '';
$class = empty($categorySEF) ? ' class="current"' : '';
echo '<li><a'.$class.' href="'._SITE.'">'.l('home').'</a></li>';
$class = ($categorySEF == 'archive') ? ' class="current"' : '';
echo '<li><a'.$class.' href="'._SITE.'archive/">'.l('archive').'</a></li>';
$query = "SELECT id, seftitle, title FROM "._PRE.'articles'." WHERE position = 3 $qwr ORDER BY artorder ASC, id";
$result = mysql_query($query);
$num = mysql_num_rows($result);
while ($r = mysql_fetch_array($result)) {
$title = $r['title'];
$class = ($categorySEF == $r['seftitle'])? ' class="current"' : '';
if ($r['id'] != s('display_page')) {
echo '<li><a'.$class.' href="'._SITE.$r['seftitle'].'/">'.$title.'</a></li>';
}
}
$class = ($categorySEF == 'contact') ? ' class="current"': '';
echo '<li><a'.$class.' href="'._SITE.'contact/">'.l('contact').'</a></li>';
$class = ($categorySEF == 'sitemap') ? ' class="current"': '';
echo '<li><a'.$class.' href="'._SITE.'sitemap/">'.l('sitemap').'</a></li>';
if ($num) $_No3 = true;
}
*/

?>


What changes here and what improvements over skians function?
1. Firstly skian's LI function is built into the pages function - I think it should be on it's own as it can work for other functions.  I built it in that way where it can do this.
2. Against the original, each built-in page is made into an array to be built into a main array to be checked against the $categorySEF.  Instead of checking against each item every 2 lines, or one by Skian, my refactor does the whole array as you would for the db array

Now, if you are using my mod here, which puts the archive, sitemap, and contact pages in the database, then this code would be much smaller.  Even further, I would like to see no true built in pages and everything can be called from the db, even the home page (this is what I have in my testing version of snLite).

 - off topic - wtf does $qwr supposed to stand for? I changed the variable to $vis (quake coding)
« Last Edit: October 09, 2009, 12:02:29 PM by nukpana »
Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Improve one function a week
« Reply #32 on: October 09, 2009, 01:38:07 AM »

Fred - I thought I fixed that here: http://snewscms.com/forum/index.php?topic=7996.0 I think that would solve that problem and some other problems noted frequently, but some people need to realize the current functionality hinders usability as you noted.

Jason, you may well have fixed it, but --as far as I know-- that fix isn't in the official download package? That's what I meant by "making it into the core". At least, in the version I downloaded last (around mid-july), your fix wasn't included. So, I'd have to remember your fix and incorporate it (or simply move the archive myself, which I do). Which shouldn't be needed, imho. Unless of course I'm the only one who hates, erm, dislikes having user-made pages pop up in between system pages. ;)

Well, it is not in the official download because the 'fix' is not considered a fix for a bug - having an extra show on the home page only - which is not considered a bug by some team members.  This was decided as this wasn't the intended functionality of extra so it is not a bug - I highly beg to differ. But I digress. 

No I dislike the fact that any system pages are not customizable and you have to go through hoops to hide it, sort it, add descriptions to it, etc, when if you just allow the system to do it's thing, then you wouldn't need to hack it like this.
« Last Edit: October 09, 2009, 01:39:48 AM by nukpana »
Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Improve one function a week
« Reply #33 on: October 09, 2009, 02:34:42 AM »

New improved function:

Admin categories - 2 queries down to 1 and no html_input

Code: [Select]
<?php

// CATEGORIES - ADMIN LIST
function admin_categories() {
$tab 1;
$link _SITE.'?action=admin_category';
?>

<div class="adminpanel">
<fieldset>
<legend><?php echo l('categories'); ?> - <a href="admin_category/"><?php echo l('add_new'); ?></a></legend>
<form method="post" action="?action=process&amp;task=reorder" id="post" accept-charset="<?php echo s('charset'); ?>">
<p>
<input type="hidden" name="order" id="order" value="snews_categories" />
</p>
<?php 
        
// solution found here: http://stackoverflow.com/questions/1281723/mysql-query-to-order-by-parent-then-child-places
        // Modified for usage here
$query "
SELECT p.id, p.name, p.description, p.published, p.catorder, p.subcat  
FROM "
._PRE."categories as p
ORDER BY p.catorder, (
SELECT LPAD(parent.id, 5, '0') 
    FROM "
._PRE."categories as parent 
    WHERE parent.id = p.id 
     AND parent.subcat = 0 
UNION
    SELECT CONCAT(LPAD(parent.id, 5, '0'), '.', LPAD(child.id, 5, '0')) 
FROM "
._PRE."categories as parent 
INNER JOIN "
._PRE."categories child 
ON (parent.id = child.subcat) 
WHERE child.id = p.id 
AND parent.subcat = 0 
)"

$result mysql_query($query);
if ( !$result || !mysql_num_rows$result ) ) {
echo '<p>'.l('category_not_exist').'</p>';
} else {
while ( $r mysql_fetch_array$result ) ) {
$subClass = ($r['subcat'] != 0) ?  ' class="subcat"' '';
?>

<p<?php echo $subClass?>>
<input type="text" name="cat_<?php echo $r['id']; ?>" value="<?php echo $r['catorder']; ?>" size="1" tabindex="<?php echo $tab?>" /> &nbsp;
<strong><?php echo $r['name']; ?></strong>
<?php echo l('divider'); ?>
<a href="<?php echo $link?>&amp;id=<?php echo $r['id']; ?>" title="<?php echo $r['description']; ?>">
<?php echo l('edit'); ?>
</a>
<?php
echo $r['published'] != 'YES' 
' '.l('divider').' ['.l('status').' '.l('unpublished').']' 
'';
echo ( $r['subcat'] == 
?  ' ' l('divider') .
<a href="' 
.$link .'&amp;sub_id=' $r['id'] . '" title="' $r['description'] . '">
'
.l('add_subcategory').'
</a>'
'';
?>

</p>
<?php 
$tab++;
}
}
?>

<p>
<input type="submit" name="reorder" id="reorder" class="button" value="<?php echo l('order_content'); ?>" />
</p>
</form>
</fieldset>
</div>
<?php
}

?>

« Last Edit: October 09, 2009, 12:00:20 PM by nukpana »
Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Improve one function a week
« Reply #34 on: October 09, 2009, 03:16:46 AM »

Maybe not *that* much of an improvement, but clear up a little bit of memory and eliminate *some* bytes....

Code: [Select]
if (isset($_POST['save'])) {
$ufield = array(
'website_title' => $_POST['website_title'],
'home_sef' => $_POST['home_sef'],
'website_description' => $_POST['website_description'],
'website_keywords' => $_POST['website_keywords'],
'website_email' => $_POST['website_email'],
'contact_subject' => $_POST['contact_subject'],
'language' => $_POST['language'],
'charset' => $_POST['charset'],
'date_format' => $_POST['date_format'],
'article_limit' => $_POST['article_limit'],
'rss_limit' => $_POST['rss_limit'],
'display_page' => $_POST['display_page'],
'comments_order' => $_POST['comments_order'],
'comment_limit' => $_POST['comment_limit'],
'word_filter_file' => $_POST['word_filter_file'],
'word_filter_change' => $_POST['word_filter_change'],
'display_new_on_home' => $_POST['display_new_on_home'],
'display_pagination' => $_POST['display_pagination'],
'num_categories' => $_POST['num_categories'],
'show_cat_names' => $_POST['show_cat_names'],
'approve_comments' => $_POST['approve_comments'],
'mail_on_comments' => $_POST['mail_on_comments'],
'word_filter_enable' => $_POST['word_filter_enable'],
'enable_extras' => (($_POST['enable_extras'] == 'on')
? 'YES'
: 'NO'),
'enable_comments' => (($_POST['enable_comments'] == 'on')
? 'YES'
: 'NO'),
'freeze_comments' => (($_POST['freeze_comments'] == 'on')
? 'YES'
: 'NO'),
'comment_repost_timer' => (is_numeric($_POST['comment_repost_timer'])
? $_POST['comment_repost_timer']
: '15'),
'file_extensions' => $_POST['file_ext'],
'allowed_files' => $_POST['allowed_file'],
'allowed_images' => $_POST['allowed_img']
);

All the variables created here are gone, as there was no real need for them.., just link the $_POST data
Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Improve one function a week
« Reply #35 on: October 09, 2009, 03:33:44 AM »

Admin Grouping... 2 queries to 1.  Stats query was really unnecessary.  If there are no groups then don't add a add new link?

Code: [Select]
<?php

// ADMIN GROUPINGS
/* Original Function
function admin_groupings() {
  if (s('enable_extras') == 'YES') {
if (stats('extras','') > 0) {
$add = ' - <a href="admin_groupings/" title="'.l('add_new').'">'.l('add_new').'</a>';
} else {
$add = '';
}
echo '<div class="adminpanel"><fieldset><legend>'.l('groupings').$add.'</legend>';
$result = mysql_query('SELECT id,name,description FROM '._PRE.'extras'.' ORDER BY id ASC');
if (!$result || !mysql_num_rows($result)) {
echo '<p>'.l('group_not_exist').'</p>';
} else {
while ($r = mysql_fetch_array($result)) {
echo '<p><strong>'.$r['name'].'</strong> '.l('divider').'<a href="'._SITE.'?action=admin_groupings&amp;id='.$r['id'].'" title="'.$r['description'].'">'.l('edit').'</a></p>';
  }
  }
  echo '</fieldset></div>';
}
}
*/
// ADMIN GROUPINGS
function admin_groupings() {
 
if (s('enable_extras') == 'YES') {
 
echo '
  <div class="adminpanel">
  <fieldset>
  <legend>
  '
.l('groupings').' - <a href="admin_groupings/" title="'.l('add_new').'">'.l('add_new').'</a>
  </legend>'
;
  
$query 'SELECT id, name, description FROM '._PRE.'extras'.' ORDER BY id ASC';
 
$result mysql_query$query );
if (!$result || !mysql_num_rows($result)) {
echo '
<p>
'
.l('group_not_exist').'
</p>'
;
} else {
while ($r mysql_fetch_array($result)) {
echo '
<p>
<strong>'
.$r['name'].'</strong> 
'
.l('divider').'
<a href="'
._SITE.'?action=admin_groupings&amp;id='.$r['id'].'" title="'.$r['description'].'">
'
.l('edit').'
</a>
</p>'
;
 
}
  }
  echo '
  </fieldset>
 </div>'
;

}

?>

« Last Edit: October 09, 2009, 03:40:23 AM by nukpana »
Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Improve one function a week
« Reply #36 on: October 09, 2009, 10:38:00 AM »

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)

Further Reduce the number of queries -  the first select isn't needed.  Not fully tested.

Code: [Select]
<?php

function delete_cat($id){
    
$delQuery "DELETE FROM "._PRE."categories WHERE id = $id LIMIT 1"
    
mysql_query$delQuery );
    
$upQuery "UPDATE "._PRE."categories 
     SET catorder = catorder - 1 
     WHERE catorder > (
     SELECT catorder 
FROM "
._PRE.'categories'.
WHERE id = 
$id)
     AND subcat = (
     SELECT subcat 
FROM "
._PRE.'categories'.
WHERE id = 
$id)";
   
  mysql_query$upQuery );
}

?>

« Last Edit: October 09, 2009, 11:23:10 AM by nukpana »
Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Improve one function a week
« Reply #37 on: October 09, 2009, 10:56:16 AM »

Future Posting:  2 updates to 1

Code: [Select]
<?php

// ARTICLES - FUTURE POSTING
/* Original Function
function update_articles() {
$last_date = s('last_date');
$updatetime = !empty($last_date) ? strtotime($last_date) : time();
$dif_time = time() - $updatetime;
if ($dif_time > 1200 || empty($last_date)) {
mysql_query('UPDATE '._PRE.'articles'.'
SET published=1
WHERE published=2
AND date <= NOW()');
mysql_query('UPDATE '._PRE.'settings'.'
SET value=NOW()
WHERE name=\'last_date\'');
}
}
*/

function update_articles() {
$last_date s('last_date');
$updatetime = !empty( $last_date 
strtotime$last_date 
time();
$dif_time time() - $updatetime;
if ( $dif_time 1200 || empty( $last_date ) ) {
$updateQuery 'UPDATE articles as a, settings as s 
SET a.published = 1, s.value = NOW()
WHERE a.published = 2
AND s.name = "last_date"'
;
mysql_query$updateQuery );
}
}

?>

Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Improve one function a week
« Reply #38 on: October 09, 2009, 11:22:39 AM »

Not fully tested but it works for me...

Function check_if_unqiue

Code: [Select]
<?php

/*
// Original Function
// CHECK IF UNIQUE
function check_if_unique($what, $text, $not_id = 'x', $subcat) {
$text = clean($text);
switch ($what) {
case 'article_seftitle':
$sql = _PRE.'articles'.' WHERE seftitle = "'.$text.(!empty($not_id) ? '"
AND category = '.$not_id : '"');
break;
case 'article_title':
$sql = _PRE.'articles'.' WHERE title = "'.$text.(!empty($not_id) ? '"
AND category = '.$not_id : '"');
break;
case 'subcat_seftitle':
$sql = _PRE.'categories'.' WHERE seftitle = "'.$text.'"
AND subcat = '.$subcat;
break;
case 'subcat_name':
$sql = _PRE.'categories'.' WHERE name = "'.$text.'"
AND subcat = '.$subcat;
break;
case 'cat_seftitle_edit':
$sql = _PRE.'categories'.' WHERE seftitle = "'.$text.'"
AND id != '.$not_id;
break;
case 'cat_name_edit':
$sql = _PRE.'categories'.' WHERE name = "'.$text.'"
AND id != '.$not_id;
break;
case 'subcat_seftitle_edit':
$sql = _PRE.'categories'.' WHERE seftitle = "'.$text.'"
AND subcat = '.$subcat.' AND id != '.$not_id;
break;
case 'subcat_name_edit':
$sql = _PRE.'categories'.' WHERE name = "'.$text.'"
AND subcat = '.$subcat.' AND id != '.$not_id;
break;
case 'group_seftitle':
$sql = _PRE.'extras'.' WHERE seftitle = "'.$text.(!empty($not_id) ? '"
AND id != '.$not_id : '"');
break;
case 'group_name':
$sql = _PRE.'extras'.' WHERE name = "'.$text.(!empty($not_id) ? '"
AND id != '.$not_id : '"');
break;
}
$rows = mysql_num_rows(mysql_query('SELECT id FROM '.$sql));
if ($rows == 0) {
return false;
} else {
return true;
}
}
*/

// Proposed Replacement
function checkUnique$table$column$text$not_id ''$subcat '' ) {
$text clean$text );
if ( !empty( $not_id ) ) {
$not_id = ( $table == 'articles' )
' AND category = '.$not_id
' AND id != '.$not_id;
}
if ( !empty( $subcat ) ) $subcat ' AND subcat = '.$subcat;
$sql 'SELECT id FROM ' _PRE $table ' WHERE ' $column .' = "' $text '" ' $subcat $not_id;
$rows mysql_num_rowsmysql_query$sql ) );
if ($rows == 0) return false;
else return true;
}

?>


What this does:

Example:
check_if_unique('article_title', $title, $category );
checkUnique('article' 'title', $title, $category );

We are removing the case statements and calling the table and column in the function call allowing for smaller and cleaner code
Logged

Keyrocks

  • Doug
  • ULTIMATE member
  • ******
  • Karma: 449
  • Posts: 6019
  • Semantically Challenged
    • snews.ca
Re: Improve one function a week
« Reply #39 on: October 09, 2009, 03:11:06 PM »

off topic - wtf does $qwr supposed to stand for? I changed the variable to $vis (quake coding)

Also off-topic... who re-wrote the queries (in sNews 1.6) for sNews 1.7? Whoever did might know what $qwr stands for. My guess is - "Query With Request"? Next question: what does $vis stand for?
« Last Edit: October 09, 2009, 04:25:47 PM by Keyrocks »
Logged
Do it now... later may not come.
-------------------------------------------------------------------------------------------------
sNews 1.6 MESU | sNews 1.6 MEMU

Joost

  • Guest
Re: Improve one function a week
« Reply #40 on: October 09, 2009, 03:27:12 PM »

off topic - wtf does $qwr supposed to stand for? I changed the variable to $vis (quake coding)

Also off-topic... who re-wrote the queries for sNews 1.7? Whoever did might know what $qwr stands for. My guess is - "Query With Request"? Next question: what does $vis stand for?


queries in the backend have never, ever been rewritten as I stated before.
Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Improve one function a week
« Reply #41 on: October 09, 2009, 03:38:43 PM »

@ doug - from memory, all the $qwr variables are all for the if ADMIN then add " AND visible = YES" line to the query.
so in this context $vis is short for Visible, what does $qwr supposed to mean?

@ Joost - I forgot where you mentioned this and not sure why they weren't done for 1.7 (memory is not serving me well this morning).  .
Logged

Keyrocks

  • Doug
  • ULTIMATE member
  • ******
  • Karma: 449
  • Posts: 6019
  • Semantically Challenged
    • snews.ca
Re: Improve one function a week
« Reply #42 on: October 09, 2009, 04:27:41 PM »

off topic - wtf does $qwr supposed to stand for? I changed the variable to $vis (quake coding)

Also off-topic... who re-wrote the queries for sNews 1.7? Whoever did might know what $qwr stands for. My guess is - "Query With Request"? Next question: what does $vis stand for?

queries in the backend have never, ever been rewritten as I stated before.

I should have been more specific... as in moving to the query structure used in 1.7 rather than re-using the structure used in 1.6.
Logged
Do it now... later may not come.
-------------------------------------------------------------------------------------------------
sNews 1.6 MESU | sNews 1.6 MEMU

Joost

  • Guest
Re: Improve one function a week
« Reply #43 on: October 09, 2009, 05:05:42 PM »

@ Joost - I forgot where you mentioned this and not sure why they weren't done for 1.7 (memory is not serving me well this morning). 

Backend (admin pages/processing)  was not rewritten because:

- It was known that most dudes working on the project lack basic knowledge of sql.
Drastic changes in the backend would make it hard (for some of us impossible) to add new features (Orderizer mod for instance) to the system.

- Reducing the number of queries there, requires a lot of work with little effect: There are always much more visitors (hopefully) than administrators (probably no more than 1 a day).

I should have been more specific... as in moving to the query structure used in 1.7 rather than re-using the structure used in 1.6.

I haven't got a clue what you're trying to say.
Logged

Keyrocks

  • Doug
  • ULTIMATE member
  • ******
  • Karma: 449
  • Posts: 6019
  • Semantically Challenged
    • snews.ca
Re: Improve one function a week
« Reply #44 on: October 09, 2009, 06:00:14 PM »

I should have been more specific... as in moving to the query structure used in 1.7 rather than re-using the structure used in 1.6.

I haven't got a clue what you're trying to say.

No big deal; I don't want to stretch this off-topic, but you do know that d-base queries are written differently (useing unfamiliar variables) in 1.7 than they were in 1.6... which means making the transition from 1.6 to 1.7 increases the 'learning curve' for sNews users who want to apply their 1.6 mods to 1.7. This is the main reason I hadn't taken time to work with 1.7 until recently.

I still don't know what $qwr (and a few others) stands for.  :P
Logged
Do it now... later may not come.
-------------------------------------------------------------------------------------------------
sNews 1.6 MESU | sNews 1.6 MEMU
Pages: 1 2 [3] 4 5 ... 9