SitePoint Sponsor

User Tag List

Results 1 to 19 of 19
  1. #1
    SitePoint Wizard
    Join Date
    Dec 2005
    Posts
    1,738
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    How do I stop repeating code in similar functions?

    Hi all

    I'm working on a bit of Drupal 7 theming, generally modifying a few fields, nothing to complex.
    One thing that is frustrating me is I keep repeating 80% of the same code in each function as shown below for a particular content type.

    What is the best practice, how do I combine this code/functions so I don't DRY ?
    The only things different are the function names and the first span class.

    PHP Code:
    function theme1_field__field_event_date_and_time__event($variables) {
      
    $output '';
      
    $output .= '<span class="time"></span>';
      
      
    // Render the items.
      
    $output .= '<div class="field-items"' $variables['content_attributes'] . ' style="display:inline-block">';
      foreach (
    $variables['items'] as $item) {
        
    $classes 'field-item';
        
    $output .= '<div class="' $classes '"' $variables['item_attributes'] . '>' drupal_render($item) . '</div>';
      }
      
    $output .= '</div>';
      
      
    // Render the top-level DIV.
      
    $output '<li class="list-group-item"><div class="' $variables['classes'] . '"' $variables['attributes'] . '>' $output '</div></li>';
      
      return 
    $output;
    }

    function 
    theme1_field__field_event_location__event($variables) {
      
    $output '';
      
    $output .= '<span class="map-marker"></span>';
      
      
    // Render the items.
      
    $output .= '<div class="field-items"' $variables['content_attributes'] . ' style="display:inline-block">';
      foreach (
    $variables['items'] as $item) {
        
    $classes 'field-item';
        
    $output .= '<div class="' $classes '">' drupal_render($item) . '</div>';
      }
      
    $output .= '</div>';
      
      
    // Render the top-level DIV.
      
    $output '<li class="list-group-item"><div class="' $variables['classes'] . '"' $variables['attributes'] . '>' $output '</div></li>';
      
      return 
    $output;

    Any suggestions on the best way to go about this?
    Thanks, Barry
    The more you learn.... the more you learn there is more to learn.

  2. #2
    SitePoint Wizard
    Join Date
    Dec 2005
    Posts
    1,738
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Could I write some sort of class here, or would that be a bit overkill?
    Maybe with different parameters?

    Any ideas?

    I'm also wondering, is there a simple way to make numerous functions use the same code, like in CSS?

    example:
    .bigtext, .smalltext {color:white}

    couldn't we have:
    function somename1 (), function somename2() {
    ....
    }

    ?
    The more you learn.... the more you learn there is more to learn.

  3. #3
    SitePoint Wizard
    Join Date
    Dec 2005
    Posts
    1,738
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    To simplify things:

    PHP Code:
    function theme1_field__field_event_date_and_time__event($variables) {
      
    $output '';
      
    $output .= '<span class="time"></span>';

    //10+ lines of duplicate code

    }

    function 
    theme1_field__field_event_location__event($variables) {
      
    $output '';
      
    $output .= '<span class="map-marker"></span>';

    //10+ lines of duplicate code


    Can you see the problem?

    Thanks, Barry
    The more you learn.... the more you learn there is more to learn.

  4. #4
    SitePoint Mentor bronze trophy
    John_Betong's Avatar
    Join Date
    Aug 2005
    Location
    City of Angels
    Posts
    1,889
    Mentioned
    74 Post(s)
    Tagged
    6 Thread(s)
    Functions can call other functions.

    Try this:

    PHP Code:
    <?php 
    define 
    ('SPC''&nbsp;&nbsp&nbsp;');

    $variables date('H:i:s');

    echo 
    '<br /><br />';
    $var theme1_field__field_event_date_and_time__event($variables);
    echo 
    $var;

    echo 
    '<br /><br />';
    $var theme1_field__field_event_location__event($variables);
    echo 
    $var;

    // following common functions could/should be in an include file
    // include './pathToCommonfunctin/commonFunctions.php';
    function theme1_field__field_event_date_and_time__event($param 'No $time passing')
    {
      
    $result '<b>function: </b>' .__FUNCTION__;

      
    $result .= '<span class="time"></span>'

      
    $result .= '<br />'
      
    // 10+ lines of duplicate code 
      
    $param   =  SPC .'<b>source: </b>theme1_field__field_event_date_and_time__event($variables)'
               
    '<br />' .SPC .SPC .'<b>time: </b>' .SPC .$param;  
      
    $result .= tenLinesOfDuplicateCode$param );


      return 
    $result;


    function 
    theme1_field__field_event_location__event($param 'No $time passing')

      
    $result '<b>function: </b>' .__FUNCTION__;

      
    $result .= '<span class="map-marker"></span>'

      
    $result .= '<br />'
      
    //10+ lines of duplicate code 
      
    $param   SPC .'<b>source: </b> theme1_field__field_event_location__event($variables)'
               
    '<br />' .SPC .SPC .'<b>time: </b>' .SPC .$param;  
      
    $result .= tenLinesOfDuplicateCode$param );

      return 
    $result;
    }  

    function 
    tenLinesOfDuplicateCode$param 'Default: No time specified')
    {
        
    $result SPC .'<b>function: </b>' .__FUNCTION__ ;

      
    $result .= '<br />' .SPC .'<b>contents: </b>'
        for(
    $i2=1$i2 10$i2++)
        {
            
    $result .= 'line: ' .$i2 .', &nbsp; '// with a comma
        
    }
      
    $result .= 'line: ' .$i2// no comma

      
    $result .= '<br />' .SPC .$param;

      return 
    $result;
    }
    Output:

    function: theme1_field__field_event_date_and_time__event
    function: tenLinesOfDuplicateCode
    contents: line: 1, line: 2, line: 3, line: 4, line: 5, line: 6, line: 7, line: 8, line: 9, line: 10
    source: theme1_field__field_event_date_and_time__event($variables)
    time: 22:58:34

    function: theme1_field__field_event_location__event
    function: tenLinesOfDuplicateCode
    contents: line: 1, line: 2, line: 3, line: 4, line: 5, line: 6, line: 7, line: 8, line: 9, line: 10
    source: theme1_field__field_event_location__event($variables)
    time: 22:58:34
    Last edited by John_Betong; Jun 5, 2014 at 10:07. Reason: formatting and added Output
    Learn how to be ready for The New Move to Discourse

    How to make Make Money Now with a *NEW* look

    Be sure to congratulate Wolfshade on earning Member of the Month for August 2014

  5. #5
    SitePoint Wizard
    Join Date
    Dec 2005
    Posts
    1,738
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Cool thanks John

    Though this seems more confusing than my original code, the amount of code is still the same, no?

    Would a class be better?
    Here is a small snippet of a class I'm using for something else, wondering if we can use this approach:

    PHP Code:
    class NodeFieldQuery extends EntityFieldQuery {
      function 
    __construct($node_type$order_by) { 
    PHP Code:
    $query = new NodeFieldQuery('event','field_event_date_and_time'); 
    Can you see what I've done here, wouldn't this be a little easier and less code?

    I'm still learning PHP as I'm sure you can tell, information much appreciated thanks.

    Barry
    The more you learn.... the more you learn there is more to learn.

  6. #6
    SitePoint Mentor bronze trophy
    John_Betong's Avatar
    Join Date
    Aug 2005
    Location
    City of Angels
    Posts
    1,889
    Mentioned
    74 Post(s)
    Tagged
    6 Thread(s)
    Hi Barry,

    There are many different ways to code and no doubt your technique and style will change with more practise.

    If all you want to do is reduce the duplication then try this. Please note the commonFunction() is verbose because I find it a lot easier to quickly scan and find errors.

    PHP Code:
     <?php define ('SPC''&nbsp;&nbsp&nbsp;');

    $variables = array 
    (
      
    'content_date'       => date('H:i:s'),
      
    'content_attributes' => 'content_attributes',
      
    'items'              => array('One','two','three'),
      
    'item_attributes'    => 'item_attributes',
      
    'classes'            => 'classes',
      
    'attributes'         => 'attributes',
    );
    echo 
    '<pre>'print_r($variables); echo '</pre>';

    echo 
    '<br />' .theme1_field__field_event_date_and_time__event($variables);

    echo  
    '<br />' .theme1_field__field_event_location__event($variables);


    //=================================
    function theme1_field__field_event_date_and_time__event($variables)
    {
      echo  
    '<b>function: </b>' .__FUNCTION__ .'<br />';;

      
    $output '<span class="time"></span>';

      
    $output .= commonFunction($variables);  

      return 
    $output;
    }

    //=================================
    function theme1_field__field_event_location__event($variables)
    {
      echo  
    '<b>function: </b>' .__FUNCTION__ .'<br />';

      
    $output '<span class="map-marker"></span>';
      
      
    $output .= commonFunction($variables);  
      
      return 
    $output;
    }  

    //=================================
    function commonFunction($variables)  
    {
      echo  
    SPC .'<b>function: </b>' .__FUNCTION__ .'<br />';

      
    $output '';

      
    // Render the items.
      
    $output .= '<div class="field-items"' 
              
    .     $variables['content_attributes'
              .     
    ' style="display:inline-block">';

      foreach (
    $variables['items'] as $item)
      {
        
    $classes 'field-item';
        
    $output .= '<div class="' 
                
    .   $classes 
                
    .    '"' 
                
    .    $variables['item_attributes'
                .  
    '>' 
                
    .    'drupal_render($item)' 
                
    '</div>';
      }
      
    $output .= '</div>';
      
      
    // Render the top-level DIV.
      
    $output .= '<li class="list-group-item"><div class="' 
              
    .    $variables['classes'
              .   
    '"' 
              
    .   $variables['attributes'
              .   
    '>' 
              
    .    $output 
              
    '</div></li>';

      return 
    $output;
    }


    //=================================
    function drupal_render($var)
    {
      echo  
    SPC .'<b>function: </b>' .__FUNCTION__ .'<br />';

      return 
    $var
    }
    Classes are a whole new ball game, have fun

    John
    Last edited by John_Betong; Jun 5, 2014 at 11:42. Reason: formatting
    Learn how to be ready for The New Move to Discourse

    How to make Make Money Now with a *NEW* look

    Be sure to congratulate Wolfshade on earning Member of the Month for August 2014

  7. #7
    Community Advisor bronze trophy
    fretburner's Avatar
    Join Date
    Apr 2013
    Location
    Brazil
    Posts
    1,446
    Mentioned
    45 Post(s)
    Tagged
    13 Thread(s)
    The only actual difference between the two functions seems to be the class name that's applied to the span element, so why not just do something like this?
    PHP Code:
    function theme1_field__field_event__event($type$variables) { 
        
    $output ''
        
    $output .= "<span class=\"$type\"></span>"

        
    //10+ lines of duplicate code 

    I'm not familiar with the Drupal naming convention for theme functions, but you'd basically want to make it generic so that it applies to any event type, with the type passed as the first parameter.
    "There are two ways of constructing a software design: One way is to make it so simple that there are obviously no deficiencies and the other way is to make it so complicated that there are no obvious deficiencies."

  8. #8
    SitePoint Wizard
    Join Date
    Dec 2005
    Posts
    1,738
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Thanks John
    There are many different ways to code and no doubt your technique and style will change with more practise.
    If all you want to do is reduce the duplication then try this.
    Code examples much appreciated, hopefully once I get a bit more established as you say
    I like your coding style here.

    --------------

    Hi fretburner
    Yes this is exactly the kind of thing I'm thinking of:
    PHP Code:
    function theme1_field__field_event__event($type$variables) {  
        
    $output '';  
        
    $output .= "<span class=\"$type\"></span>";  

        
    //10+ lines of duplicate code  

    The only actual difference between the two functions seems to be the class name that's applied to the span element...
    At the moment yes, though I need the two different function names as these function names are unique to the drupal fields, I also have a third function which also has the same code excluding the <span>. So in total we have:
    PHP Code:
    function theme1_field__event($variables) {
    // this is generally what the event fields default to
    }
    function 
    theme1_field__field_event_location__event($variables) {
    }
    function 
    theme1_field__field_event_date_and_time__event($variables) {

    What do you suggest now?

    you'd basically want to make it generic so that it applies to any event type, with the type passed as the first parameter
    And could you elaborate on this.

    FYI

    theme_1 = the name of our theme
    field = we're targeting a field (can be other things like, block or view)
    field_event_location = the unique field name
    event = the content type name

    Putting all that together = theme1_field__field_event_location__event
    The more you learn.... the more you learn there is more to learn.

  9. #9
    SitePoint Wizard bronze trophy
    Join Date
    Jul 2006
    Location
    Augusta, Georgia, United States
    Posts
    4,192
    Mentioned
    17 Post(s)
    Tagged
    4 Thread(s)
    You would just call the function which you copied the code from. If the file that function exists in hasn't been included yet than you would need to manually call drupal_load_include to bring in the file.

    PHP Code:
    function theme1_field__field_event_location__event($variables) {
      
    $output foobar($variables); // you need to figure out what this function is
      
    $out 'mystuff'.$out;
      return 
    $out;

    Though there is likely a *better way to achieve your end goal than doing that. I would have to understand the whole scope to actually recommend anything else.
    The only code I hate more than my own is everyone else's.

  10. #10
    Community Advisor bronze trophy
    fretburner's Avatar
    Join Date
    Apr 2013
    Location
    Brazil
    Posts
    1,446
    Mentioned
    45 Post(s)
    Tagged
    13 Thread(s)
    I've just seen that oddz already posted a similar reply, but as I've already written a response I'll post it anyway in case it's useful.

    PHP Code:
    function theme1_field__event($variables)
    {
        return 
    build_event_item($variables);
    }

    function 
    theme1_field__field_event_location__event($variables)
    {
        
    $prefix '<span class="map-marker"></span>';
        return 
    build_event_item($variables$prefix);
    }

    function 
    theme1_field__field_event_date_and_time__event($variables)
    {
        
    $prefix '<span class="time"></span>';
        return 
    build_event_item($variables$prefix);
    }

    function 
    build_event_item($variables$prefix_markup '')
    {
        
    // Render the items.
        
    $items '';
        foreach (
    $variables['items'] as $item) {
            
    $classes 'field-item';
            
    $items .= '<div class=' $classes'>' drupal_render($item) . '</div>';
        }

        
    // Render the containing markup.
        
    $output = <<<EOD
        <li class="list-group-item">
          <div class="
    {$variables['classes']}{$variables['attributes']} >
            
    $prefix_markup
            <div class="field-items" 
    {$variables['content_attributes']} style="display:inline-block">
              
    $items
            </div>
          </div>
        </li>
    EOD;

        return 
    $output;

    "There are two ways of constructing a software design: One way is to make it so simple that there are obviously no deficiencies and the other way is to make it so complicated that there are no obvious deficiencies."

  11. #11
    SitePoint Wizard
    Join Date
    Dec 2005
    Posts
    1,738
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    It works fretburner
    I see what you have done here, this is exactly the type of approach I was referring to.

    Likewise with oddz suggestion which I tried first though the <span> elements were being printed outside of the <li>, not sure if I was doing it correctly.
    I'll digest all this properly tomorrow and get back with updates, very tired now

    Also hoping to see what oddz suggests once I explain a little further, maybe we can fine tune this even more

    Thanks again all for your time.

    Chat soon, Barry
    The more you learn.... the more you learn there is more to learn.

  12. #12
    SitePoint Wizard
    Join Date
    Dec 2005
    Posts
    1,738
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Though there is likely a *better way to achieve your end goal than doing that. I would have to understand the whole scope to actually recommend anything else.
    I'm general just changing/adding a bit of markup, new attributes to some fields inside my event content type.

    The only other alternative I could see oddz was to create a load of template files field--field-event-date-and-time.tpl.php and so on.
    I was under the impression that if possible, its better to make the changes inside your template.php, faster ?

    What other ways are there?
    I'll soon have more fields I need to change, things can soon add up.
    Is there a best approach, wouldn't my template.php soon become heavy with code for making tweaks to fields, or would I be better creating template files?

    Nice to hear you view on this and how you do things yourself.

    And thanks again fretburner, currently using your method.

    Thanks,
    Barry
    The more you learn.... the more you learn there is more to learn.

  13. #13
    SitePoint Wizard bronze trophy
    Join Date
    Jul 2006
    Location
    Augusta, Georgia, United States
    Posts
    4,192
    Mentioned
    17 Post(s)
    Tagged
    4 Thread(s)
    Why do you need to add additional attributes and change the markup in the first place.
    The only code I hate more than my own is everyone else's.

  14. #14
    SitePoint Wizard
    Join Date
    Dec 2005
    Posts
    1,738
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    So I can customise my site more. Sometimes I'll need to add spans and other html elements, custom graphics, microdata, data-attributes amongst other things.
    Isn't this what the theming layer is for so we can build our own theme to how we like it?

    So whats best, template.php or .tpl.php ?
    A mixture or do you recommend a different approach oddz?
    And are you saying what I'm doing above is wrong?

    I just want to make sure I'm doing things correctly before I dive into a big build.
    Appreciate your feedback thanks.

    Cheers, Barry
    The more you learn.... the more you learn there is more to learn.

  15. #15
    SitePoint Wizard bronze trophy
    Join Date
    Jul 2006
    Location
    Augusta, Georgia, United States
    Posts
    4,192
    Mentioned
    17 Post(s)
    Tagged
    4 Thread(s)
    It's difficult to speak on a generic level about this topic. Could you provide some concrete examples of what your actually trying to accomplish from a design standpoint.
    The only code I hate more than my own is everyone else's.

  16. #16
    SitePoint Wizard
    Join Date
    Dec 2005
    Posts
    1,738
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    In a nutshell oddz,
    I'd just like to understand the best way of modifying fields, best practice, writing less code and speed.
    Generally, how to add extra markup and attributes to fields when I need extra customisation per theme for individual fields.

    Cheers, Barry
    The more you learn.... the more you learn there is more to learn.

  17. #17
    SitePoint Wizard bronze trophy
    Join Date
    Jul 2006
    Location
    Augusta, Georgia, United States
    Posts
    4,192
    Mentioned
    17 Post(s)
    Tagged
    4 Thread(s)
    Well to be honest I've never needed to override the HTML output of a field provided by core or a contrib module. Having said that I have created my own custom fields, and used token replacement in views to customize HTML. I've also created custom view modes and displays for fields to solve other problems. There are many different ways to achieve things in Drupal. Though on a generic level it is difficult to recommend any one approach. Typically though if you have to repeat code like you have done there is a better means of solving the problem at hand. Typically… not always.
    The only code I hate more than my own is everyone else's.

  18. #18
    SitePoint Wizard
    Join Date
    Dec 2005
    Posts
    1,738
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    There are many different ways to achieve things in Drupal.
    As I'm finding out

    I have created my own custom fields, and used token replacement in views to customize HTML
    I have come across tokens though haven't really used them yet, generally been learning and working with code, rather than the admin.

    Though on a generic level it is difficult to recommend any one approach
    Might sound like a silly question, but what exactly do you mean when you say 'generic level' slightly confusing me, what does it mean?

    Thanks, Barry
    The more you learn.... the more you learn there is more to learn.

  19. #19
    SitePoint Wizard bronze trophy
    Join Date
    Jul 2006
    Location
    Augusta, Georgia, United States
    Posts
    4,192
    Mentioned
    17 Post(s)
    Tagged
    4 Thread(s)
    I mean why do you need to add attributes or classes. Do you need them as hooks for other libraries to apply css or javascript perhaps.
    The only code I hate more than my own is everyone else's.


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •