patternphpMinor
Rendering articles with breadcrumb navigation
Viewed 0 times
withnavigationbreadcrumbarticlesrendering
Problem
I'm wondering if perhaps I am doing this all wrong:
Questions:
-
Should I create separate methods to use for the following line:
Notes:
Further Information to help provide insight:
`-[Folder] Lib
Questions:
- How is my OOP?
- Is it running multiple methods on a value {i.e.: is
strip_tags($copy->truncateString($articles[$i]['body'], 250, " "))} a terrible way to manage resources?
-
Should I create separate methods to use for the following line:
echo '' . $articles[$i]['title'] . ' (' . $articles[$i]['date'] . ')' . strip_tags($copy->truncateString($articles[$i]['body'], 250, " ")) . ' Read more';Notes:
pageTemplateis a class which will contain all HTML templates for this particular site
- Two static classes take care of site wide configuration variables (not seen here) and tracking the URL (
BreadCrumbs::).
- Calling
BreadCrumbs::getCrumb(x)retrieves the 'value' at the x position of the url (ie: domain.com/"pos 1"/"pos 2"/"pos 3"). The values are managed at the top of the page and are checked if empty and replaced with a default value if they are.
GetCopyis a class which has a number of functions designed to run a query (based on various parameters) and retrieves an array (or mulch-dimensional array). The function used below retrieves a mulch-dimensional array with each second level array containing a whole article.
GetCopyhas a parent. It extends a class calledCopyManwhich contains various functions such as adding paragraphs to a block of text, truncating, converting date forms, and pagination tools, called through theGetCopyobject.
/**
* homePage :: ()
* PUBLIC method
* = lay out the home page content
*
*/
public static function homePage() {
// initialize GetCopy class
$copy = new GetCopy();
// special feature container
echo '';
$articles = $copy->getRows('articles', 'date', 0, 2);
//print_r($articles);
for ($i = 0; $i ' . $articles[$i]['title'] . ' (' . $articles[$i]['date'] . ')' . strip_tags($copy->truncateString($articles[$i]['body'], 250, " ")) . ' Read more';
}
echo '';
}Further Information to help provide insight:
`-[Folder] Lib
Solution
I have a few good suggestions for you.
Max Line Length
Try to cut down on the length of your lines, at my current resolution(running from a cheap laptop on the train) stackexchange displays 90 characters per code line, your longest lines are far above 300, for sharing code try to keep it manageable.
Foreach
You're using a for loop for an array, which is fine, but the foreach function is more efficient if you're iterating all objects, and makes things a little cleaner.
Abstraction
In this instance I think you should abstract your display, maybe make a class to handle that stuff?
Put logic where it belongs
Put simply put the act of displaying the article in your display class, and leave the logic where it is.
Demo of what I think it should look like
Max Line Length
Try to cut down on the length of your lines, at my current resolution(running from a cheap laptop on the train) stackexchange displays 90 characters per code line, your longest lines are far above 300, for sharing code try to keep it manageable.
Foreach
You're using a for loop for an array, which is fine, but the foreach function is more efficient if you're iterating all objects, and makes things a little cleaner.
Abstraction
In this instance I think you should abstract your display, maybe make a class to handle that stuff?
Put logic where it belongs
Put simply put the act of displaying the article in your display class, and leave the logic where it is.
Demo of what I think it should look like
public static function homePage() {
$copy = new GetCopy();
$display = DisplayEngine::GetInstance();
$display->printSpecialHeader();
$display->openSpecialContainer();
$articles = $copy->getRows('articles', 'date', 0, 2);
foreach( $articles AS $article ){
$display->printArticleLink($article);
}
$display->closeSpecialContainer();
}
...
class DisplayEngine {
public function printArticleLink($article){
echo ''
. htmlspecialchars($article['title'])
. ' ('
. $article['date']
. ')'
. $copy->truncateString(
strip_tags($article['body']),
250,
" "
)
. ' Read more';
}
}Code Snippets
public static function homePage() {
$copy = new GetCopy();
$display = DisplayEngine::GetInstance();
$display->printSpecialHeader();
$display->openSpecialContainer();
$articles = $copy->getRows('articles', 'date', 0, 2);
foreach( $articles AS $article ){
$display->printArticleLink($article);
}
$display->closeSpecialContainer();
}
...
class DisplayEngine {
public function printArticleLink($article){
echo '<div class="special_summary"><a href="/'
. BreadCrumbs::getCrumb(1)
. '/'
. BreadCrumbs::getCrumb(2)
. '/article/'
. $article['id']
. '"><h5>'
. htmlspecialchars($article['title'])
. '</h5></a> ('
. $article['date']
. ')<p>'
. $copy->truncateString(
strip_tags($article['body']),
250,
" "
)
. '</p><p><a href="/'
. BreadCrumbs::getCrumb(1)
. '/'. BreadCrumbs::getCrumb(2)
. '/article/'
. $article['id']
. '"> Read more</a></p></div>';
}
}Context
StackExchange Code Review Q#19888, answer score: 2
Revisions (0)
No revisions yet.