Please login or register.

Login with username, password and session length
Advanced search  

News:

Latest sNews - sNews 1.7 - with its own forums - for discussion and user mods.

Pages: 1 ... 7 8 [9]

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

skian

  • Full Member
  • ***
  • Karma: 14
  • Posts: 120
Re: Improve one function a week
« Reply #120 on: October 07, 2010, 03:07:04 PM »

Sorry, it's been updated
Why introduce alternative syntax for control statements ?

Btw, I'd rather remove all l('divider') with is a bullet that should be implemented with css
Logged

nukpana

  • Hero Member
  • *****
  • Karma: 71
  • Posts: 663
Re: Improve one function a week
« Reply #121 on: October 07, 2010, 08:56:59 PM »

Sorry, it's been updated
Why introduce alternative syntax for control statements ?
TBH, I am not sure.  Perhaps I was coding for a different system at the time and stuck with it's way... I dunno.  It's not wrong is it?
Logged

skian

  • Full Member
  • ***
  • Karma: 14
  • Posts: 120
Re: Improve one function a week
« Reply #122 on: December 22, 2010, 11:11:38 PM »

The following code is meant to replace the obscure code from lines 123 to 221 (in sn171).

Code: [Select]
<?php

function getUrlInformation($urlstring) {
/*
This function parses $urlstring & return an array containing metatdata about the query

Urlstring should be of the form
/ Category / subcategory / article / 
/ Category / subcategory / article / c_xxx
/ Category / subcategory / article / p_xxx
/ Category / subcategory  /  
/ Category / article /
/ Category / 
/ Page /

*/

global $commentsPage,$pageNum;

$URI explode('/'clean($urlstring));
while(
count($URI)>&& empty($URI[0])) array_shift($URI);
while(
count($URI)>&& empty($URI[count($URI)-1])) unset($URI[count($URI)-1]);
if (
count($URI)>&& strpos($URI[count($URI)-1], l('comment_pages'))===&& is_numeric(substr($URI[count($URI)-1], strlen(l('comment_pages'))))) {
$commentsPage substr($URI[count($URI)-1], strlen(l('comment_pages')));
unset($URI[count($URI)-1]);
}
if (
count($URI)>&& strpos($URI[count($URI)-1], l('paginator'))===&& is_numeric(substr($URI[count($URI)-1], strlen(l('paginator'))))) {
$pageNum substr($URI[count($URI)-1], strlen(l('paginator')));
unset($URI[count($URI)-1]);
}

if (_ADMIN) {
$pub_a ''$pub_c ''$pub_x '';
} else {
$pub_a ' AND a.published = 1';
$pub_c ' AND c.published =\'YES\'';
$pub_x ' AND x.published =\'YES\'';
}

switch(
count($URI)) {
case 
3:
$MainQuery 'SELECT
a.id AS id, title, position, description_meta, keywords_meta,
c.id AS catID, c.name AS name, c.description, x.name AS xname
FROM '
._PRE.'articles'.' AS a,
'
._PRE.'categories'.' AS c
LEFT JOIN '
._PRE.'categories'.' AS x
ON c.subcat=x.id
WHERE a.category=c.id
'
.$pub_a.$pub_c.$pub_x.'
AND x.seftitle="'
.$URI[0].'"
AND c.seftitle="'
.$URI[1].'"
AND a.seftitle="'
.$URI[2].'"
'
;
break;
case 
2:
$MainQuery 'SELECT
a.id AS id, title, position, description_meta, keywords_meta,
c.id as catID, name, description, subcat, NULL
FROM '
._PRE.'articles'.' AS a
LEFT JOIN '
._PRE.'categories'.' AS c
ON category =  c.id
WHERE c.seftitle = "'
.$URI[0].'"
AND a.seftitle ="'
.$URI[1].'"
'
.$pub_a.$pub_c.'
AND subcat = 0
UNION

SELECT
NULL as id, NULL, 0, NULL, NULL, c.id AS catID, c.name AS name, c.description, c.subcat,
x.name AS xname
FROM '
._PRE.'categories'.' AS x
LEFT JOIN '
._PRE.'categories'.' AS c
ON  c.subcat = x.id
WHERE x.seftitle = "'
.$URI[0].'"
AND c.seftitle = "'
.$URI[1].'"
'
.$pub_c.$pub_x ;
break;
case 
1:
$MainQuery 'SELECT
a.id as id, title, description_meta, keywords_meta, position, category as catID,  name, description
FROM '
._PRE.'articles'.' AS a
LEFT JOIN '
._PRE.'categories'.' AS c
ON category =  c.id
WHERE a.seftitle = "'
.$URI[0].'"
'
.$pub_a.'
AND position = 3
UNION
SELECT
NULL, NULL, NULL, NULL, 0, c.id AS catID, name, description
FROM '
._PRE.'categories'.' AS c
WHERE c.seftitle = "'
.$URI[0].'"
AND subcat = 0
'
.$pub_c;

break;
case 
0:
$MainQuery 'SELECT
id, title, category, description_meta, keywords_meta, position
FROM '
._PRE.'articles'.' AS a
WHERE id = "'
.s('display_page').'"
'
.$pub_a.'
AND position = 3'
;
break;
default:
return null;
}

$q mysql_query($MainQuery);
$D mysql_fetch_assoc($q);

if (
$D === false) {
// ethier page not found or builtin page

if(in_array($URI[0],explode(',',l('cat_listSEF')))){
$D['categorySEF'] = $URI[0];
} else {
$D['categorySEF'] = '404';
header('HTTP/1.1 404 Not Found');
}


return 
$D;
}

$R getUrlInformation($_GET['category']);
update_articles();
if(!empty(
$R['categorySEF'])) $categorySEF $R['categorySEF'];

?>

Logged

Fred K

  • Still trying to learn stuff
  • ULTIMATE member
  • ******
  • Karma: 130
  • Posts: 2728
    • Personal
Re: Improve one function a week
« Reply #123 on: August 03, 2011, 12:53:48 AM »

I'm a little annoyed at Patch #19 in 1.7.1. Not because it was introduced, and certainly not annoyed at those who made it - some of it was necessary - but because a lot of work that had already been done was obliterated. I'll expand. Back on page 5 in this topic, I began a complete overhaul of the admin panels' html output. The purpose of the overhaul was to make the admin panels easier to read and thereby easier to work with for front-end users. Patch 19 changed that, and for the most part not in a good way. I'll try and explain.

1. Originally the Administration panels in 1.7 used <form><fieldset> in panels that had no form elements. To improve that we replaced the <form><fieldset> construction with a <div>. I also introduced a clearer structure with headings separating each function block to increase usability. Patch 19 changed that clear structure to an unclear structure by removing headings and using only paragraphs instead - this decreases usability and should be fixed.

Example: Original refactoring:
Code: [Select]
<div class="adminpanel">
<h3>'.l('comments').'</h3>
<p><a onclick="toggle(\'sub1\')" style="cursor: pointer;" title="'.l('unapproved').'">'.$unapproved.' '.l('wait_approval').'</a></p>

Patch 19
Code: [Select]
echo '<div class="adminpanel">
<p class="admintitle">'.l('comments').'</p>';
echo '<p><a onclick="toggle(\'sub1\')" style="cursor: pointer;" title="'.l('unapproved').'">'.$unapproved.' '.l('wait_approval').'</a></p>';

I do not understand the change from headings to paragraphs for admin titles. These are headings and should be marked up as such.

2. There were originally a number of panels which do have form elements, panels which benefit from having form elements grouped in fieldsets. Work to improve the use of fieldset groups for those panels was also done in the major overhaul, and again that work was removed in Patch 19 where *all* fieldsets have been replaced with divs. Not only was that change unnecessary for the most part, but it also removes a level of structure (and thereby readability) that improved the original 1.7 version.

3. There are a lot of instances of
echo '<p>'.$something.'</p>;
echo '<p>'.$next_thing.'</p>';
echo '<p>'.$and_one_more_for_good_measure.'</p>';
, lines that could (should) be written more effectively, like
echo '<p>'.$something.'</p>
<p>'.$next_thing.'</p>
<p>'.$and_one_more_for_good_measure.'</p>';
, something that was done in the original overhaul. Sure, it's a minor detail but it affects the output and for frontend developers this is just as important as the efficiency of the backend.

As noted at the start, I don't want this to be taken as criticism aimed at any person, and it's not criticsm based on the fact that it happened to be me what did the original refactoring - I just don't understand why changes were made through Patch 19 that aren't improvements, changes that actually removed improvements which had already been made. I'm all for changes if they make things better but in this case many of the changes in the admin panels' html output were not improved by Patch 19 - on the contrary.

But maybe no-one rally cares about these details.
Logged

Keyrocks

  • Doug
  • ULTIMATE member
  • ******
  • Karma: 449
  • Posts: 6019
  • Semantically Challenged
    • snews.ca
Re: Improve one function a week
« Reply #124 on: August 03, 2011, 01:51:03 AM »

I'm a little annoyed at Patch #19 in 1.7.1. Not because it was introduced, and certainly not annoyed at those who made it - some of it was necessary - but because a lot of work that had already been done was obliterated. I'll expand.

Good vent, Fred. Maybe I had something to do with Patch #19, and maybe not, it's been so long since I've looked at the distro file-set. I suppose, at this point in time, it doesn't matter how it occured or who did it.

I'm not at my office machine ATM but I seem to recall that the "official" download package had validated the last time I checked it.

By all means, if you feel there are improvements to be made, please feel free to modify a fresh copy of the download and either one of the Admins can refresh the upload with your patched version... 1.7.2.
Logged
Do it now... later may not come.
-------------------------------------------------------------------------------------------------
sNews 1.6 MESU | sNews 1.6 MEMU

Fred K

  • Still trying to learn stuff
  • ULTIMATE member
  • ******
  • Karma: 130
  • Posts: 2728
    • Personal
Re: Improve one function a week
« Reply #125 on: August 03, 2011, 12:45:23 PM »

Doug, it doesn't matter who, that's completely unimportant - it's the what. And the "vent" has nothing to do with validation - I'm sure the output HTML validates fine, and I'm fairly sure not many would bother validating the admin area anyway, but that's not what this is about. It's about having a structurally sound html output that's not just efficient but makes it easier for admins to work with in the visual admin area as well as easier to debug just by checking the source code in the browser. I know, details, schmetails, but they do count for something.

Also...
Quote from: Keyrocks
if you feel there are improvements to be made, please feel free to modify a fresh copy of the download

Of course. Except I already did that (there's a thread about it in the dudes' area as well, from around the same time). And look what happened...
You'll be able to get the modified administration code with my new themes though. "Soon", as it were.
« Last Edit: August 03, 2011, 12:49:46 PM by Fred K »
Logged
Pages: 1 ... 7 8 [9]