patternphpMinor
Drupal: Placing Code (PHP) within the Drupal System (as well as css)
Viewed 0 times
drupalplacingthecssphpsystemwithinwellcode
Problem
I have a node template that grabs fields from a content type.
In the node--[contenttype].tpl.php file I have two sections (The second is html/php and grabs variables from the first section or from the content type and simply displayed it).
The first is strictly php and does three things:
My question is, should this php even go in the node--[contenttype].tpl.php file? The numbers output and taxonomy output is only ever going to be used in this node (the arrays set for comparison are only relevant with this content type), but the Class function could be used on other pages.
I'm not getting how I can leverage the Drupal system in order to get a strong and easily upgradable code base (for going to Drupal 8 in the future).
How should my mind be processing executable challenges. Should I be asking certain questions in order to determine if something goes into template.php, into a new module, or where its css goes? Should all that css be going into style.css? Can I break it out somehow?
EDIT: Here is my code for node--[contenttype].tpl.php
```
34,
"Curriculum" => 35,
"Study Labs" => 36,
"Stage Performances" => 37,
);
$location_to_nid = array(
"Visit Examples" => 38,
"We Come To You" => 39);
// to array (from stdClass object)
// for field_location_category to get name
// without it, get white screen of death because ['taxonomy_term'] is an object not array
// Credit: http://www.if-not-true-then-false.com/2009/php-tip-convert-stdclass-object-to-multidimensio
In the node--[contenttype].tpl.php file I have two sections (The second is html/php and grabs variables from the first section or from the content type and simply displayed it).
The first is strictly php and does three things:
- Grabs numbers from one of the content type fields and rearranges them according to some criteria. Setting this output as a variable to be used later in the page.
- Grabs two taxonomy terms and sets them as variables to be used later in the page. Also compares these variables to an array as the key in order to set the value as another variable to be used later in the page.
- Creates a function for converting stdClass Objects to Arrays and uses it in one of the above two actions.
My question is, should this php even go in the node--[contenttype].tpl.php file? The numbers output and taxonomy output is only ever going to be used in this node (the arrays set for comparison are only relevant with this content type), but the Class function could be used on other pages.
I'm not getting how I can leverage the Drupal system in order to get a strong and easily upgradable code base (for going to Drupal 8 in the future).
How should my mind be processing executable challenges. Should I be asking certain questions in order to determine if something goes into template.php, into a new module, or where its css goes? Should all that css be going into style.css? Can I break it out somehow?
EDIT: Here is my code for node--[contenttype].tpl.php
```
34,
"Curriculum" => 35,
"Study Labs" => 36,
"Stage Performances" => 37,
);
$location_to_nid = array(
"Visit Examples" => 38,
"We Come To You" => 39);
// to array (from stdClass object)
// for field_location_category to get name
// without it, get white screen of death because ['taxonomy_term'] is an object not array
// Credit: http://www.if-not-true-then-false.com/2009/php-tip-convert-stdclass-object-to-multidimensio
Solution
I can't speak specifically to Drupal as I have never used it before. But the following should be helpful in any situation. In order to best answer your specific questions I'm going to have to go through this line by line, which means I need to decipher your code first. Since I'm having to do so anyways, I figured I'd share the results as a review, after all, this is code review.
Style
Be consistent with your style/code. You have two arrays at the very beginning, both are styled slightly differently. Either method is fine, but you should stick to one or the other. Mixing styles makes it appear to be copy-pasted together.
Arrays and Constants
I would have to ask what these arrays are for that they are sequentially ordered in this way. Skimming the rest of this document I see that these arrays are only used once, and are used in such a way that they can be combined. Honestly I think these should actually be refactored into constants, but I can't really see how to do this to demonstrate. However, combining them definitely wont hurt.
Comments
Now, I kind of skipped over the first thing I wanted to cover, but let me return to it now. There are three different kinds of comments. Your single line comments
Adopted Code
Its good to give credit to borrowed code, especially when trying to look it up later, but don't just copy-paste it in. Take the time to manually type it into your own code, and tweak it to your own style and habits while you are at it. This ensures seamless integration and will help you, if not understand it, to at least know its inner workings better. For instance, you use very descriptive names for all of your variables, but in that function you use a very vague variable. In fact, it is just good practice to be descriptive anyways. This makes your code self documenting, making many of these comments unnecessary.
Don't Repeat Yourself
There is a principle called "Don't Repeat Yourself" (DRY). As the name implies, you should do your best to make sure your code is not repetitive. Sometimes this means using loops, sometimes variables, and usually it means using functions. But to fully benefit from this principle you should use them all. I'm not going to discuss each method here, but you should definitely take a look at this principle more in depth. Specifically I want to look at variables here. Variables help make your code DRY by ensuring you don't have to type the same sequences repeatedly. This also reduces the line length of your code, for example:
This has the added benefit of making your code easier to edit. Imagine your URL changed. You would then have to update that URL everywhere. Sure, you can use the find/replace tool, but only having to do it once ensures that nothing is accidentally missed AND it makes your code smaller.
Conditions
So a neat little trick here. If you have a variable you want to set to TRUE/FALSE, based on the outcome of a condition, you can just pass that condition to the variable for the same results, for example:
However, while this is a neat trick, it is unnecessary in this context, because the variables are unnecessary. You only use these variables once. So instead of performing the same expression twice, just use those conditions once, when you actually need them.
Echo vs Print
This is a matter of preference, but typically you see
Alternative Syntax
Every statement/loop has an alternate syntax that is used in templating. This alternate syntax uses a colon instead of the opening brace, and an
```
$count = count( $und );
for( $i = 0; $i < $count; $i++ ) {
$tid_array[] = $und[ $i ] [ 'tid'
Style
Be consistent with your style/code. You have two arrays at the very beginning, both are styled slightly differently. Either method is fine, but you should stick to one or the other. Mixing styles makes it appear to be copy-pasted together.
Arrays and Constants
I would have to ask what these arrays are for that they are sequentially ordered in this way. Skimming the rest of this document I see that these arrays are only used once, and are used in such a way that they can be combined. Honestly I think these should actually be refactored into constants, but I can't really see how to do this to demonstrate. However, combining them definitely wont hurt.
$category_to_nid = array(
"Professional Development" => 34,
"Curriculum" => 35,
"Study Labs" => 36,
"Stage Performances" => 37,
"Visit Examples" => 38,
"We Come To You" => 39
);Comments
Now, I kind of skipped over the first thing I wanted to cover, but let me return to it now. There are three different kinds of comments. Your single line comments
//single line comment, your multi-line comments / multi-line comment /, and finally your doccomments /** doccomment */. While this doesn't really effect performance, it does help with legibility when you use them properly.//set link and text for Category and Location section
/*
to array (from stdClass object)
for field_location_category to get name
etc...
*/Adopted Code
Its good to give credit to borrowed code, especially when trying to look it up later, but don't just copy-paste it in. Take the time to manually type it into your own code, and tweak it to your own style and habits while you are at it. This ensures seamless integration and will help you, if not understand it, to at least know its inner workings better. For instance, you use very descriptive names for all of your variables, but in that function you use a very vague variable. In fact, it is just good practice to be descriptive anyways. This makes your code self documenting, making many of these comments unnecessary.
Don't Repeat Yourself
There is a principle called "Don't Repeat Yourself" (DRY). As the name implies, you should do your best to make sure your code is not repetitive. Sometimes this means using loops, sometimes variables, and usually it means using functions. But to fully benefit from this principle you should use them all. I'm not going to discuss each method here, but you should definitely take a look at this principle more in depth. Specifically I want to look at variables here. Variables help make your code DRY by ensuring you don't have to type the same sequences repeatedly. This also reduces the line length of your code, for example:
$und = $node->field_location_category[ 'und' ];
$location_text_array = objectToArray( $und[ 0 ] [ 'taxonomy_term' ] );
//AND
$address = 'http://www.server.com/node/';
$location_lin = $address . $location_to_nid[ $location_text ];This has the added benefit of making your code easier to edit. Imagine your URL changed. You would then have to update that URL everywhere. Sure, you can use the find/replace tool, but only having to do it once ensures that nothing is accidentally missed AND it makes your code smaller.
Conditions
So a neat little trick here. If you have a variable you want to set to TRUE/FALSE, based on the outcome of a condition, you can just pass that condition to the variable for the same results, for example:
$display_category_tab = $category_text != $category_text_previous;However, while this is a neat trick, it is unnecessary in this context, because the variables are unnecessary. You only use these variables once. So instead of performing the same expression twice, just use those conditions once, when you actually need them.
if( $category_text != $category_text_previous ) {
print "Category TAB" . "";
}Echo vs Print
This is a matter of preference, but typically you see
echo being used over print. Both are just fine, but echo seems to be a smidge faster, which I guess is the reason its more widely used. Otherwise there is no difference, at least that I know of.Alternative Syntax
Every statement/loop has an alternate syntax that is used in templating. This alternate syntax uses a colon instead of the opening brace, and an
endXXX instead of the closing brace. As I said, this syntax is used in templating, so seeing it in a the middle of a PHP block is jarring. I would use the traditional syntax instead.```
$count = count( $und );
for( $i = 0; $i < $count; $i++ ) {
$tid_array[] = $und[ $i ] [ 'tid'
Code Snippets
$category_to_nid = array(
"Professional Development" => 34,
"Curriculum" => 35,
"Study Labs" => 36,
"Stage Performances" => 37,
"Visit Examples" => 38,
"We Come To You" => 39
);//set link and text for Category and Location section
/*
to array (from stdClass object)
for field_location_category to get name
etc...
*/$und = $node->field_location_category[ 'und' ];
$location_text_array = objectToArray( $und[ 0 ] [ 'taxonomy_term' ] );
//AND
$address = 'http://www.server.com/node/';
$location_lin = $address . $location_to_nid[ $location_text ];$display_category_tab = $category_text != $category_text_previous;if( $category_text != $category_text_previous ) {
print "Category TAB" . "<br />";
}Context
StackExchange Code Review Q#16398, answer score: 3
Revisions (0)
No revisions yet.