Sloppy script... how would you clean it up?

[cgm225]

I have the following procedural code that I use to generate my blog/note pages of my website.  It utilizes a notes class I created, and also gets information from variables stored in my MVC registry.  However, it seems really messy to me, and I feel like there is a better way to code this so it appears "neater."  I just feel like it is sloppy procedural code...  how would you improve it?

 

<?php

echo "\t\t<div class=\"text_container\">\n";
echo "\t\t\t<h2 class=\"notes\">Notes:: More from me..</h2>\n";
echo "\t\t\tI use this area of my site to share random thoughts, experiences, etc.\n\t\t\t<br />\n\n";

echo "\t\t\t<div class=\"note_navigation_container\">\n";
echo "\t\t\t\t<div class=\"note_navigation_right\">\n";

//If there are more pages beyond the current one, a "Next" link is provided
if ($this->registry->get('totalEntries') > $this->registry->get('lastPageEntry')) { 
    echo "\t\t\t\t\t<a href=\"http://" . $_SERVER['SERVER_NAME'] . "/notes/" . ($this->registry->get('page') + 1) . "\">Next</a>\n";
}

echo "\t\t\t\t</div>\n";
echo "\t\t\t\t<div class=\"note_navigation_left\">\n";

//If the user is beyond the first page, a "Previous" link is provided
if ($this->registry->get('page') > 1) {
    echo "\t\t\t\t\t<a href=\"http://" . $_SERVER['SERVER_NAME'] . "/notes/" . ($this->registry->get('page') - 1) . "\">Previous</a>\n";
} else {
    echo "\t\t\t\t\t<span style=\"color:#ffffff;\">_</span>\n";
}

echo "\t\t\t\t</div>\n";
echo "\t\t\t\t<div class=\"note_navigation_center\">\n\t\t\t\t\t";

//Calculating the starting pagination number
$startingPaginationNumber = $this->registry->get('page') - 2;

//If the user is on page four or beyond, a link to page one is provided
if ($startingPaginationNumber <= 1) {
    $startingPaginationNumber = 1;
} else {
    echo "<a href=\"http://" . $_SERVER['SERVER_NAME'] . "/notes/1\" title=\"First Page\">«</a> ";
}

//Calculating the ending pagination number
$endingPaginationNumber = $this->registry->get('page') + 2;

if ($endingPaginationNumber > $this->registry->get('totalPages')) {
    $endingPaginationNumber = $this->registry->get('totalPages');
}

/* Navigation via a numerical index will be provided with links directly to the
* two most adjacent pages on either side of the current page number, as well as
* the additional links to the starting and ending pages,  all such that the
* output will resemble "<< 3 4 5 6 7 >>" where 5 is the current page 
*/
for ($i = $startingPaginationNumber; $i <= $endingPaginationNumber; $i++) {
    if ($i == $this->registry->get('page')) {
        echo "<span style=\"font-weight:800;text-decoration:underline;\">" . $i . "</span> ";
    } else {
        echo "<a href=\"http://" . $_SERVER['SERVER_NAME'] . "/notes/$i\">$i</a> ";
    }
}

/* If the user is three or more pages from the end, a link to the last page is
* provided
*/ 
if ($endingPaginationNumber < $this->registry->get('totalPages')) {
    echo "<a href=\"http://" . $_SERVER['SERVER_NAME'] . "/notes/" . $this->registry->get('totalPages') . "\" title=\"Last Page\">»</a>";
}

echo "\n\t\t\t\t</div>\n";
echo "\t\t\t</div>\n\n";

//Outputting entry data
foreach($this->registry->get('notes') as $note) {
    echo "\t\t\t<div class=\"notes\">\n";
    echo "\t\t\t\t<h2 class=\"notes\">" . $note['title'] . "</h2>\n";
    echo "\t\t\t\t" . $note['date'] . "\n";
    echo "\t\t\t\t<div class=\"notes_body\">\n\t\t\t\t\t" . nl2br(stripslashes($note['note'])) . "\n\t\t\t\t</div>";
    echo "\n\t\t\t</div>\n";
}

echo "\t\t</div>";

?>

[jonsjava]

took me a while, but here's a "cleaner version"

<?php
$server_name = $_SERVER['SERVER_NAME'];
echo <<<END
		<div class="text_container">
		<h2 class="notes">Notes:: More from me..</h2>
		I use this area of my site to share random thoughts, experiences, etc.
		<br />


		<div class="note_navigation_container">
			<div class="note_navigation_right">
END;
//If there are more pages beyond the current one, a "Next" link is provided
if ($this->registry->get('totalEntries') > $this->registry->get('lastPageEntry')) { 
$page_plus_one = $this->registry->get('page') + 1;
echo <<<END
				<a href=\"http://$server_name/notes/$page_plus_one">Next</a>

END;
}
echo <<<END
				</div>
				<div class="note_navigation_left">
END;
//If the user is beyond the first page, a "Previous" link is provided
if ($this->registry->get('page') > 1) {
$page_minus_one = $this->registry->get('page') - 1;
echo <<<END
				<a href="http://$server_name/notes/$page_minus_one">Previous</a>
END;
} else {
echo <<<END
					<span style="color:#ffffff;">_</span>
END;
}
echo <<<END
			</div>
			<div class="note_navigation_center">

END;
//Calculating the starting pagination number
$startingPaginationNumber = $this->registry->get('page') - 2;

//If the user is on page four or beyond, a link to page one is provided
if ($startingPaginationNumber <= 1) {
    $startingPaginationNumber = 1;
} else {
echo <<<END
<a href="http://$server_name/notes/1" title="First Page">«</a>
END;
}

//Calculating the ending pagination number
$endingPaginationNumber = $this->registry->get('page') + 2;

if ($endingPaginationNumber > $this->registry->get('totalPages')) {
    $endingPaginationNumber = $this->registry->get('totalPages');
}

/* Navigation via a numerical index will be provided with links directly to the
* two most adjacent pages on either side of the current page number, as well as
* the additional links to the starting and ending pages,  all such that the
* output will resemble "<< 3 4 5 6 7 >>" where 5 is the current page 
*/
for ($i = $startingPaginationNumber; $i <= $endingPaginationNumber; $i++) {
    if ($i == $this->registry->get('page')) {
    	echo <<<END
    	<span style="font-weight:800;text-decoration:underline;">$i</span>
END;
    } else {
    	echo <<<END
    	<a href="http://$server_name/notes/$i">$i</a>
END;
    }
}

/* If the user is three or more pages from the end, a link to the last page is
* provided
*/ 
if ($endingPaginationNumber < $this->registry->get('totalPages')) {
$total_pages = $this->registry->get('totalPages');
echo <<<END
<a href="http://$server_name/notes/$total_pages" title="Last Page">»</a>
END;
}
echo <<<END

			</div>
		</div>


END;

//Outputting entry data
foreach($this->registry->get('notes') as $note) {
$title = $note['title'];
$date = $note['date'];
$br_note = nl2br(stripslashes($note['note']));
    echo <<<END
		<div class="notes">
    				<h2 class="notes">$title</h2>
    				$date
    				<div class="notes_body">
    					$br_note
    				</div>
END;
}
echo <<<END
	</div>
END;
?>

[cooldude832]

are those tabs (\t) the mess?

They are actually there to keep uniform tabbing output so the output can be cleanly analyzed

 

Its not messy at all just using a tabbing method described above achieves the same principle.

[jonsjava]

echo <<<END will keep tabs in place. I agree. tabs aren't messy, unless you plan on changing code often.

[MasterACE14]

I fix up sloppy code, or third party scripts using this website. http://www.phpformatter.com/

 

Regards ACE

[Guest Xanza]

Actually you're off using the original format... Although JohnsJava's code looks more clean, it will actually parse much more slowly. He uses heredoc to keep tab positions and to execute code, and in the PHP manual you can plainly read that the heredoc function is the SLOWEST php function known to php kind.

 

Sorry.

[unkwntech]

Same as jonsjava without the heredoc:

<?php
$server_name = $_SERVER['SERVER_NAME'];
?>
		<div class="text_container">
		<h2 class="notes">Notes:: More from me..</h2>
		I use this area of my site to share random thoughts, experiences, etc.
		<br />


		<div class="note_navigation_container">
			<div class="note_navigation_right">
<?php
//If there are more pages beyond the current one, a "Next" link is provided
if ($this->registry->get('totalEntries') > $this->registry->get('lastPageEntry')) { 
$page_plus_one = $this->registry->get('page') + 1;
       ?>
				<a href=\"http://$server_name/notes/<?php echo $page_plus_one; ?>">Next</a>

<?php
}
?>
				</div>
				<div class="note_navigation_left">
<?php
//If the user is beyond the first page, a "Previous" link is provided
if ($this->registry->get('page') > 1) {
$page_minus_one = $this->registry->get('page') - 1;
          ?>
				<a href="http://$server_name/notes/<?php echo $page_minus_one; ?>">Previous</a>
<?php
} else {
?>
					<span style="color:#ffffff;">_</span>
<?php
}
?>
			</div>
			<div class="note_navigation_center">

<?php
//Calculating the starting pagination number
$startingPaginationNumber = $this->registry->get('page') - 2;

//If the user is on page four or beyond, a link to page one is provided
if ($startingPaginationNumber <= 1) {
   $startingPaginationNumber = 1;
} else {
?>
<a href="http://$server_name/notes/1" title="First Page">«</a>
<?php
}

//Calculating the ending pagination number
$endingPaginationNumber = $this->registry->get('page') + 2;

if ($endingPaginationNumber > $this->registry->get('totalPages')) {
   $endingPaginationNumber = $this->registry->get('totalPages');
}

/* Navigation via a numerical index will be provided with links directly to the
* two most adjacent pages on either side of the current page number, as well as
* the additional links to the starting and ending pages,  all such that the
* output will resemble "<< 3 4 5 6 7 >>" where 5 is the current page 
*/
for ($i = $startingPaginationNumber; $i <= $endingPaginationNumber; $i++) {
   if ($i == $this->registry->get('page')) {
   	?>
   	<span style="font-weight:800;text-decoration:underline;">$i</span>
<?php
   } else {
   	?>
   	<a href="http://$server_name/notes/$i">$i</a>
<?php
   }
}

/* If the user is three or more pages from the end, a link to the last page is
* provided
*/ 
if ($endingPaginationNumber < $this->registry->get('totalPages')) {
$total_pages = $this->registry->get('totalPages');
?>
<a href="http://$server_name/notes/$total_pages" title="Last Page">»</a>
<?php
}
?>

			</div>
		</div>


<?php

//Outputting entry data
foreach($this->registry->get('notes') as $note) {
$title = $note['title'];
$date = $note['date'];
$br_note = nl2br(stripslashes($note['note']));
   ?>
		<div class="notes">
   				<h2 class="notes">$title</h2>
   				$date
   				<div class="notes_body">
   					$br_note
   				</div>
<?php
}
?>
	</div>
<?php
?>

 

Edited a cupple times 'cause i cant spell for the sake of my life.

[Guest Xanza]

Very nice, unkwntech.

[unkwntech]

Painful but it should work.

[cgm225]

Follow up question...  What do you think of the code itself?  I try to seperate my presentation logic (which is this script) from my business logic.  Have I done that?  Would you do anything differently (for example, with the pagination, the way I reference registry variables, etc.)?

 

Any feedback is greatly appreciated!

[shedokan]

I would clean it up this way:

<?php
$server_name = $_SERVER['SERVER_NAME'];
?>
	<div class="text_container">
		<h2 class="notes">Notes:: More from me..</h2>
		I use this area of my site to share random thoughts, experiences, etc.
		<br />

		<div class="note_navigation_container">
			<div class="note_navigation_right">
<?php
//If there are more pages beyond the current one, a "Next" link is provided
if ($this->registry->get('totalEntries') > $this->registry->get('lastPageEntry')) {
echo '					<a href="http://'.$server_name.'/notes/'.($this->registry->get('page') + 1).'">Next</a>';
}
?>
			</div>
			<div class="note_navigation_left">
<?php
//If the user is beyond the first page, a "Previous" link is provided
if ($this->registry->get('page') > 1) {
echo '					<a href="http://<?=$server_name; ?>/notes/<?=($this->registry->get('page') - 1); ?>">Previous</a>';
} else {
echo '					<span style="color:#ffffff">_</span>';
}
?>
			</div>
			<div class="note_navigation_center">
<?php
//Calculating the starting pagination number
$startingPaginationNumber = $this->registry->get('page') - 2;

//If the user is on page four or beyond, a link to page one is provided
if ($startingPaginationNumber <= 1) {
    $startingPaginationNumber = 1;
} else {
echo '					<a href="http://'.$server_name.'/notes/1" title="First Page">«</a>';
}

//Calculating the ending pagination number
$endingPaginationNumber = $this->registry->get('page') + 2;

if ($endingPaginationNumber > $this->registry->get('totalPages')) {
    $endingPaginationNumber = $this->registry->get('totalPages');
}

/* Navigation via a numerical index will be provided with links directly to the
* two most adjacent pages on either side of the current page number, as well as
* the additional links to the starting and ending pages,  all such that the
* output will resemble "<< 3 4 5 6 7 >>" where 5 is the current page 
*/
for ($i = $startingPaginationNumber; $i <= $endingPaginationNumber; $i++) {
    if ($i == $this->registry->get('page')) {
        echo '					<span style="font-weight:800;text-decoration:underline;">'.$i.'</span> ';
    } else {
        echo '					<a href="http://'.$_SERVER['SERVER_NAME'].'/notes/$i">'.$i.'</a> ';
    }
}

/* If the user is three or more pages from the end, a link to the last page is
* provided
*/ 
if ($endingPaginationNumber < $this->registry->get('totalPages')) {
    echo '					<a href="http://'.$_SERVER['SERVER_NAME'].'/notes/'.$this->registry->get('totalPages').'" title=\"Last Page\">»</a>';
}
?>
			</div>
		</div>
<?php
//Outputting entry data
foreach($this->registry->get('notes') as $note) {
?>
		<div class="notes">
			<h2 class="notes"><?=$note['title']; ?></h2>
			 <?=$note['date']; ?>
			<div class="notes_body">
				<?=nl2br(stripslashes($note['note'])); ?>
			</div>";
		</div>
<?php
}
?>
	</div>