What is meta? ×
Meta Stack Exchange is where users like you discuss bugs, features, and support issues that affect the software powering all 122 Stack Exchange communities.

Can anyone tell me how deliver clean code to SO when using the format as code option?

You can see my problem here. The code has a horizontal scrollbar, which is most annoying to the reader.

<!-- Simple multi image addon -->
                 <div id="fancy">
                  <table border="0" cellspacing="0" cellpadding="2" align="right">
            <tr>
              <td align="center" class="smallText">

<?php if (strlen($product_info['products_name']) > $max_title_length) {  $title = wordwrap(htmlspecialchars($product_info['products_name']), $max_title_length, '<br>');
         } else { $title = htmlspecialchars($product_info['products_name']); } 
$m_source = ''; $thumb = (class_exists('oscthumb') && CFG_MASTER_SWITCH == 'On'); if ($thumb) { preg_match('/"([^"]+)"/',htmlentities ( tep_image(DIR_WS_IMAGES . $product_info['products_image'],'','','','','',5), ENT_NOQUOTES), $image);  $m_source=str_replace('&amp;','&',$image[1]); }
  echo '<a  rel="image_group" title="'.$title.'" href="' . ($m_source ? $m_source : DIR_WS_IMAGES . $product_info['products_image']) . '" alt="' . $product_info['products_name'] . '" target="_blank">' . tep_image(DIR_WS_IMAGES . $product_info['products_image'], $product_info['products_name'], SMALL_IMAGE_WIDTH, SMALL_IMAGE_HEIGHT, 'hspace="5" vspace="5"', false, 5) . '<br /></a>'; 
               if(!$vertical_format) { echo ''; $row = 1; reset($products_image_array); foreach($products_image_array as $value) { 
                             if ($thumb) {      
                             $source = ''; preg_match('/"([^"]+)"/',htmlentities ( tep_image(DIR_WS_IMAGES . $value,'','','','','',5), ENT_NOQUOTES), $image);  $source=str_replace('&amp;','&',$image[1]); 
                             } 
                             echo '<a  rel="image_group" title="'.$title.'" href="' . ($source ? $source : DIR_WS_IMAGES . $value) . '" target="_blank">' . tep_image(DIR_WS_IMAGES . $value, $product_info['products_name'], TINY_IMAGE_WIDTH, TINY_IMAGE_HEIGHT, 'hspace="5" vspace="5"') . '</a>'; 

                                ++$row; if ($row > $image_group) { echo '<br />'; $row = 1; } } } 
                                echo '</td>';
                             if($vertical_format) { echo '<td>'; $row = 1; reset($products_image_array); foreach($products_image_array as $value) { 
                             if ($thumb) {      
                             $source = ''; preg_match('/"([^"]+)"/',htmlentities ( tep_image(DIR_WS_IMAGES . $value,'','','','','',5), ENT_NOQUOTES), $image);  $source=str_replace('&amp;','&',$image[1]); 
                             } 
                             echo '<a  rel="image_group" title="'.$title.'" href="' . ($source ? $source : DIR_WS_IMAGES . $value) . '" target="_blank">' . tep_image(DIR_WS_IMAGES . $value, $product_info['products_name'], TINY_IMAGE_WIDTH, TINY_IMAGE_HEIGHT, 'hspace="5" vspace="5"') . '<br />' . '</a>'; 

                                    ++$row; if ($row > $image_group) { echo '</td><td>'; $row = 1; } } echo '</td>'; } ?>


            </tr><?php  echo '<tr><td class="smallText">' . TEXT_CLICK_TO_ENLARGE . '</td></tr>'; ?>
          </table>
                    </div>
                    <!-- EOF Simple multi image addon -->
share|improve this question
3  
You call that code "clean"? –  Ether Oct 13 '10 at 18:15
    
@Ether no, he didn't, as said above. –  Joe the Person Oct 6 '11 at 23:34
add comment

4 Answers

Well, the problem is that you've got enormously long lines.

Reformat them to whatever width it is that SO scrolls at.

share|improve this answer
3  
Indeed, you even continue new statements after semicolons with no newline. That's not illegal, but it's certainly unusual, and I would say hard to read. Moving statements to their own lines would be an easy first step in reducing the horizontal scrollbar issue. –  Pops Oct 13 '10 at 13:27
    
On second glance, this seems to be OSCommerce code. Sadly, the whole of that product looks this way. –  Pëkka Oct 13 '10 at 14:07
3  
the problem is he pasted too much code. –  Kev Oct 13 '10 at 15:35
    
@Kev: That's certainly a lot of the problem. But you can have long lines without pasting too much code, too. –  Jon Skeet Oct 14 '10 at 7:35
add comment

Here's an answer by example. My strategy was simple: Indent consistently and add new lines after semicolons. That's all I did; I intentionally did not even read the code at all.

<!-- Simple multi image addon -->
<div id="fancy">
    <table border="0" cellspacing="0" cellpadding="2" align="right">
        <tr>
            <td align="center" class="smallText">

            <?php
            if (strlen($product_info['products_name']) > $max_title_length) {
                $title = wordwrap(htmlspecialchars($product_info['products_name']),
                    $max_title_length, '<br>');
            } else {
                $title = htmlspecialchars($product_info['products_name']);
            }

            $m_source = '';

            $thumb = (class_exists('oscthumb') && CFG_MASTER_SWITCH == 'On');

            if ($thumb) {
                preg_match('/"([^"]+)"/',htmlentities ( tep_image(DIR_WS_IMAGES . $product_info['products_image'],'','','','','',5), ENT_NOQUOTES), $image);

                $m_source=str_replace('&amp;','&',$image[1]);
            }

            echo '<a  rel="image_group" title="'.$title.'" href="' . ($m_source ? $m_source : DIR_WS_IMAGES . $product_info['products_image']) . '" alt="' . $product_info['products_name'] . '" target="_blank">' . tep_image(DIR_WS_IMAGES . $product_info['products_image'], $product_info['products_name'], SMALL_IMAGE_WIDTH, SMALL_IMAGE_HEIGHT, 'hspace="5" vspace="5"', false, 5) . '<br /></a>';

            if(!$vertical_format) {
                echo '';
                $row = 1;
                reset($products_image_array);
                    foreach($products_image_array as $value) {
                        if ($thumb) {
                            $source = '';
                            preg_match('/"([^"]+)"/',htmlentities ( tep_image(DIR_WS_IMAGES . $value,'','','','','',5), ENT_NOQUOTES), $image);
                            $source=str_replace('&amp;','&',$image[1]);
                        }
                        echo '<a  rel="image_group" title="'.$title.'" href="' . ($source ? $source : DIR_WS_IMAGES . $value) . '" target="_blank">' . tep_image(DIR_WS_IMAGES . $value, $product_info['products_name'], TINY_IMAGE_WIDTH, TINY_IMAGE_HEIGHT, 'hspace="5" vspace="5"') . '</a>';

                        ++$row;

                        if ($row > $image_group) {
                            echo '<br />';
                            $row = 1;
                        }
                    }
                } 

                echo '</td>';

                if($vertical_format) {
                    echo '<td>';
                    $row = 1;
                    reset($products_image_array);

                    foreach($products_image_array as $value) {
                        if ($thumb) {
                            $source = '';
                            preg_match('/"([^"]+)"/',htmlentities ( tep_image(DIR_WS_IMAGES . $value,'','','','','',5), ENT_NOQUOTES), $image);  $source=str_replace('&amp;','&',$image[1]);
                        }

                        echo '<a  rel="image_group" title="'.$title.'" href="' . ($source ? $source : DIR_WS_IMAGES . $value) . '" target="_blank">' . tep_image(DIR_WS_IMAGES . $value, $product_info['products_name'], TINY_IMAGE_WIDTH, TINY_IMAGE_HEIGHT, 'hspace="5" vspace="5"') . '<br />' . '</a>';

                        ++$row;

                        if ($row > $image_group) {
                            echo '</td><td>';
                            $row = 1;
                        }
                    }

                    echo '</td>';
                }
                ?>

        </tr>

        <?php
        echo '<tr><td class="smallText">' . TEXT_CLICK_TO_ENLARGE . '</td></tr>';
        ?>
    </table>
</div>
<!-- EOF Simple multi image addon -->

You'll notice that the scrollbars are still there. That's because some of your lines are long echo statements that I couldn't fix without reading and rewriting some code. Looking now, it seems that those lines contain embedded if statements and function calls. If you separate those sorts of things out into separate lines, you can shorten each individual line — and improve readability — considerably.

The best part is, this is nearly effortless. Honestly, a script could do the work that I just did, just as correctly and much more quickly. I know code formatting scripts exist for several languages, and I'd be very surprised if there wasn't one available for PHP.

share|improve this answer
1  
+1 Nice work. And for actually making the effort. –  Diago Oct 13 '10 at 13:45
3  
Great, you won another scrollbar... –  ring0 Oct 13 '10 at 15:15
add comment

Instead of posting a large wodge of code (even if it is well formatted, many people's attention spans quickly dry up, even mine), boil it down to a single atomic runnable snippet to allow other people to reproduce the problem you're experiencing.

Code such as the example you posted and even the nicely formatted version by Popular Demand will most likely go ignored because:

  1. There's just too much to comprehend
  2. It's dependant on other code or data we don't have access to or can't infer expected behaviour from

As a rule of thumb do a bit of work up front and this problem won't arise.

share|improve this answer
    
The content of my answer aside, I agree with this. Was just discussing it on another question yesterday, which you might find interesting: meta.stackoverflow.com/questions/67365/… (hm, it turned out to be a dupe.) –  Pops Oct 13 '10 at 14:51
add comment

Not an answer to your question - you presumably have an issue that you are working on, and want input for - but the root of the problem is that this is pretty bad code. No offense - I have old code lying around that looks the same. It's still bad.

If you start separating PHP and HTML, preparing and sanitizing variables in a code block, and build a coherent HTML block that you only fill stuff into and do the most basic control structures in, your code will automatically look much more pretty.

share|improve this answer
add comment

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .