2
\$\begingroup\$

I am trying to split a string so that the first word goes into the first span, and the remaining (an unknown number of words) goes into the next.

This currently works fine, but is there an easy / faster / tidier way to do it ?

<?
// Explode the job role out and combine into the two spans

    $la_jobrole = explode(' ', $lo_designer->role);
    $ls_jobPrimary = '';
    $ls_jobSecondary = '';
    $i=0;
    foreach($la_jobrole as $role) {
        if ($i == 0) $ls_jobPrimary = $role;
        else {
            $ls_jobSecondary .= $role . " ";
        }
        $i++;
    }

?>
<div class="designer"><span><?= $ls_jobPrimary ?> </span><span><?= $ls_jobSecondary ?></span></div>
\$\endgroup\$
  • \$\begingroup\$ @yuri Removing overall indentation (which is probably a result of not knowing how to properly format code on SO) is fine, but modifying the formatting of the code itself should be left for a reviewer to point out. \$\endgroup\$ – Graipher Jul 4 '17 at 10:18
3
\$\begingroup\$

Using [preg_split][1] with the limit argument will give a simpler solution without loop:

<?php
    list($ls_jobPrimary, $ls_jobSecondary) = preg_split('/\s/', $lo_designer->role, 2);
?>
<div class="designer"><span><?= $ls_jobPrimary ?> </span><span><?= $ls_jobSecondary ?></span></div>
\$\endgroup\$
  • \$\begingroup\$ Thanks this is really helpful! After a bit more research, is there an reason you would use preg_split over explode ? \$\endgroup\$ – NZSteve Jul 4 '17 at 20:36
  • \$\begingroup\$ @NZSteve: There are no specific reasons, you're right, it's possible to use explode if the space between word is really a single space. With preg_split, you can use regex, in this case, \s stands for any kind of spaces (space, tabulation ...) \$\endgroup\$ – Toto Jul 5 '17 at 10:37
2
\$\begingroup\$

explode and implode are faster (according to PHP's man page for preg_split), and more readable IMO.

$la_jobrole = explode(' ', $lo_designer->role);
$ls_jobPrimary = array_shift($la_jobrole); // Get the first item from the array
$ls_jobSecondary = implode(' ', $la_jobrole); // Glue the remaining pieces back together

Personally I would avoid using either list or preg_replace in this case. They're slower and harder to read, and I would certainly avoid that looping thing.

\$\endgroup\$
  • 1
    \$\begingroup\$ instead of using implode, use the third parameter of explode that limits the number of parts. \$\endgroup\$ – Casimir et Hippolyte Jul 12 '17 at 10:15
2
\$\begingroup\$

First, I'll offer three more methods that will provide the same/desired output, then explain some pros/cons:

Input:

$lo_designer=(object)['role'=>'Commander-in-Chief Coffee Machine Attendant'];

Output: (for all three methods to follow):

<div class="designer"><span>Commander-in-Chief </span><span>Coffee Machine Attendant</span></div>

Method #1: Limited explode()

$roles=explode(' ',$lo_designer->role,2);
?><div class="designer"><span><?= $roles[0] ?> </span><span><?= $roles[1] ?></span></div>

This is probably the most sensible approach as it provides the two desired strings with a single function call. Using list() to name the generated elements is fine if you place a high priority on speed of human comprehension, but it doesn't seem necessary to me -- given that the task is so overt. You could also implode like this:
<?= implode(' </span><span>', explode(' ', $lo_designer->role, 2)) ?>

Method #2: strstr() & substr() w/ strpos()

<div class="designer"><span><?= strstr($lo_designer->role,' ',true) ?> </span><span><?= substr($lo_designer->role,strpos($lo_designer->role,' ')+1) ?></span></div>

This approach doesn't add any new variables to the scope, but requires 3 function calls which doesn't seem very "tidy".

Method #3: Limited preg_replace()

<div class="designer"><span><?= preg_replace('/ /',' </span><span>',$lo_designer->role,1) ?></span></div>

This regex-based approach enjoys replacement limiting and only uses one set of short echo tags. However as a general rule (of mine anyhow), regex functions should only be used when other functions fail to offer a reasonably succinct equivalent. For this reason, I see no justification to use preg_split() versus explode() to achieve the same resultant array.


In conclusion, I would personally use and recommend Method #1 as it is lean and clean. Many developers advise against blending the processing and displaying portions of code into the same line as a matter of cleanliness. (Here is a demo link)

That said, I'd like to make two more points:

  • I don't like to bounce back and forth between html and php because it feels uglier to me. Other developers may consider escaping quotes and curly bracket wrapping variables to be equally ugly; or inappropriate in a template structure.

  • I wouldn't write the "role separating space" before </span>, I'd put it after since any visible css styling on the space will appear strange to the user. (If you have a good reason for writing the space inside the first span tag, fair enough.)

My Code:

$roles=explode(' ',$lo_designer->role,2);
echo "<div class=\"designer\"><span>{$roles[0]}</span> <span>{$roles[1]}</span></div>";
\$\endgroup\$
0
\$\begingroup\$

I know the others have already answered the question about techniques for "an easy / faster / tidier way to do it". I would recommend list() (like in Toto's answer but with explode() if you know for a fact that only one type of space will be used - otherwise use preg_split().

One aspect of the code I noticed was the manual incrementing of a counter:

$i=0;
foreach($la_jobrole as $role) {
    //...
    $i++;
}

This can be simplified by using the 2nd syntax of foreach:

foreach (array_expression as $key => $value)
    statement

1

When the array has numeric indexes, then there is no need to setup the counter variable and increment it at the end of each loop:

foreach($la_jobrole as $i => $role) {
    if ($i == 0) $ls_jobPrimary = $role;
    else {
        $ls_jobSecondary .= $role . " ";
    }
}

1http://php.net/manual/en/control-structures.foreach.php#control-structures.foreach

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.