views:

104

answers:

4

Currently, I need help with my loop. In each div, I want it to show two sets of FirstName and LastName instead of just one set, but I don't know how can I do that because of the loop. Also, the point of setting different font sizes is to create a visual look which is a funnel like shape. My question is, how can I add another set of name per div and is there a better way to code this? can I make my code more efficient?

Edit: Well, I'm mainly trying to figure out how to add another set of name into the div or I can just use another loop. What I mean when I say add another set of name into the div, I mean I want to add another row of data to the div; I want to have the first two rows of data fetched from MySQL in one div.

$state = 1;
$fontcount = 25;
while ($row = mysql_fetch_assoc($result)) {

 if( $fontcount == 25 ) {         $fontsize = "250%";
 } elseif( $fontcount < 25 && $fontcount >= 22 ) { $fontsize = "210%";
 } elseif( $fontcount < 22 && $fontcount >= 19 ) { $fontsize = "170%";
 } elseif( $fontcount < 19 && $fontcount >= 16 ) { $fontsize = "150%";
 } elseif( $fontcount < 16 && $fontcount >= 13 ) { $fontsize = "130%";
 } else {                     $fontsize = "110%";
 }
 if( $state%2 == 0 ) {
   echo "<div style='background-color: #black; font-size: " . $fontsize . "; text-transform:uppercase; text-align:center;'>";
 } else {
   echo "<div style='background-color: #blue; font-size: " . $fontsize . "; text-transform:uppercase; text-align:center;'>";
 }


 echo $row['FirstName'] . " " . ' <span style="font-size: 15px;">$' . $row['LastName'] . "</span>";
 echo "</div>";
 $state++;
 $fontcount--;
}
+2  A: 

What makes you think that the code is not efficient? Any speed problems in your app are almost assuredly not being caused by having an if-else ladder inside that loop!

That being said, I would have used the following:

if ($fontcount < 13)      { $fontsize = "110%"; }
else if ($fontcount < 16) { $fontsize = "130%"; }
else if ($fontcount < 19) { $fontsize = "150%"; }
else if ($fontcount < 22) { $fontsize = "170%"; }
else if ($fontcount < 25) { $fontsize = "210%"; }
else {                      $fontsize = "250%"; }

One other thing: You have two lines which are almost exactly the same. Change them to this:

my $color = ( $state%2 == 0 ) ? "black" : "blue";
echo "<div style='background-color: #" . $color . "; font-size: " . $fontsize . "; text-transform:uppercase; text-align:center;'>";

Also, (I don't know PHP, but I'm guessing) that you can embed those variables in the strings so you don't have to close the string and concatenate them all the time:

my $color = ( $state%2 == 0 ) ? "black" : "blue";
echo "<div style='background-color: #$color; font-size: $fontsize; text-transform:uppercase; text-align:center;'>";
tster
I'm just trying to learn more and see if I can pick up any new things.
Doug
That's definitely a lot easier to read, and makes things a lot simpler!
Doug
I'm all about code that looks good. I spend 8-16 hours a day looking at the stuff, it betting be pretty!
tster
+1  A: 

First, Let me get your question right...

You want to show a list of results returned from database. Display font size start large, a portion for smaller every 3 result and alternative background color for each result...

My approach is to break up your data layer with your presentation and use a bit of calculation to reduce your hardcodes.


$alt = false;
$dataset = array();
$fontcount = 25.0; // Make this float

while ($row = mysql_fetch_assoc($result)) 
{
    $datarow = array();
    $datarow['FirstName'] = $row['FirstName'];
    $datarow['LastName'] = $row['LastName']
    $datarow['FontSize'] = (int) $fontcount; // Remember to store as Integer not float
    $datarow['BackgroundColor'] = ($alt == true) ? 'black' : 'blue'; // Background color alternation
    $dataset[] = $datarow;

    $alt = ($alt == true) ? false : true; // Alternate
    $fontcount -= 0.334; // reduce float point for 1-third, therefore drop a point every 3 iteration
}

foreach($dataset in $item)
{
    $fontsize = $item['FontSize'].'0%'; // Assuming desire size is 10x of 'fontcount' in percentage
    $output = '';
    $output .= '<div style="background-color:'.$item['BackgroundColor'].'; font-size:'.$fontsize;.'text-transform:uppercase; text-align:center;">';
    $output .= $datarow['FirstName'].'<span style="font-size: 15px;">'.$datarow['LastName'].'</span>';
    $output .= '</div>';
    echo $output;
}

Also as reply to your comment, you are properbly looking for code tidiness rather then loop efficient here.

rockacola
+1  A: 

If I'm understanding your question correctly, you want to display two full names in each div.

To do that, you should only open the div tag when the record is odd, then close it when the row is even or is the last record in the set. There are a few ways to achieve this. I would suggest you keep an even/odd counter much like you're doing now with $state. Open the div tag on an odd record; close it on an even record. (Add a check after the loop to close the div if the final record was odd.) You would then have to adjust your $state variable to only increment when the record was odd, so that the divs got alternating styling.

As tster pointed out, your code here isn't really inefficient in any serious way. You could probably nit-pick a few minor things, but the only real efficiency problems you will see in most applications lies in nested loops. Since you're writing output here, I assume your code is not deeply nested, so I wouldn't sweat it.

keithjgrant
I have most of it down, except for the alternating styling part. Mind giving me a quick example?
Doug
If I adjust #state to only increment when it is odd then it will never get to odd because it won't increment on even.
Doug
I got it! I used $state for keeping track of colors and I added a new odd/even to keep track for the other part.
Doug
Yep, that's what I meant.
keithjgrant
+1  A: 

First of all, using nested ifs in my opinion should not be used when having more than 3 options. In this case i would rather do this:

switch (true) {
    case ($fontcount == 25) :
        $fontsize = "250%";
    break;
    case ($fontcount < 25 && $fontcount >= 22) :
        $fontsize = "210%";
    break;
    case ($fontcount < 22 && $fontcount >= 19) : 
        $fontsize = "170%";
    break;
    case ($fontcount < 19 && $fontcount >= 16) :
        $fontsize = "150%";
    break;
    case ($fontcount < 16 && $fontcount >= 13) :
        $fontsize = "130%";
    break;
    default:
        $fontsize = "110%";
    break;
}

When using HTML inside a pure PHP-file i generally try to stick it in variables, not to confuse myself. Also grouping or dividing the HTML makes it easier on my eyes. Such as this:

$div =  "<div style='background-color: " . 
        ($state%2 == 0 ? '#black' : '#blue') . ";" . 
        " font-size: " . $fontsize . ";" . 
        " text-transform:uppercase; text-align:center;'>";


$name = $row['FirstName'] . " " . 
        ' <span style="font-size: 15px;">$' . 
        $row['LastName'] . "</span>";

$div .= $name;
$div .= "</div>";

echo $div;

A bonus if you want shorter code, and yes i'm not saying i should do this for a living

$f=$fontcount;
$c = (($f == 25 ? 25 : ($f < 25 && $f >= 22 ? 21 : ($f < 22 && $f >= 19 ? 17 : ($f < 19 && $f >= 16 ? 15 : ($f < 16 && $f >= 13 ? 13 : 11))))) * 10) . '%';
$fontsize=$c;

To fetch two rows

while ($row = mysql_fetch_assoc($result)) {
$row2 = mysql_fetch_assoc($result);

then

if ($row2) {
    $name2 = $row2['FirstName'] . " " . 
            ' <span style="font-size: 15px;">$' . 
            $row2['LastName'] . "</span>";
}
Peter Lindqvist
`using nested ifs in my opinion should not be used when having more than 3 options` Why is this your opinion? This code is more verbose and harder to read that the OP in my opinion.
tster
"using nested ifs in my opinion should not be used when having more than 3 options." Why?
Doug
like i say, it's my opinion. for instance, the default value is clearly stated "default" in the switch statement, which you cannot say about the ifs. about verbosity well, isn't that one of the key factors for readability. but hey we are all different. i use XSLT a lot and this is nothing compare to XSLT.
Peter Lindqvist