Is there a better way to write this php code?

Question

return'
    <li>
    <a href="nano.com/' . $username . '"><img class="avatar" src="images/' . $picture . '" width="48" height="48" alt="avatar" /></a>
    <div class="tweetTxt">
    <strong><a href="nano.com/' . $username . '">' . $username . '</a></strong> '. autolink($tweet).'
    <div class="date">'.relativeTime($dt).'</div><div class="date">'. $reply_info . '</div> <a class ="reply"  href="home.php?replyto=@'. $username .'&status_id='. $id .'&reply_name=' .$username.'"> reply </a>

    </div>
    <div class="clear"></div>
    </li>';

I was wondering if there is a cleaner way to write this code, and taking in mind processing time, if that really means anything.

p.s. this code is part of a function, this is the return statement!

score 3 · Answer 1 · edited May 23 '17 at 10:32

3

Yes. Use double quotes for the PHP string (and single quotes for the HTML attributes), then you can just use PHP variables in the string, like so:

"<a href='nano.com/$username'>";

Is processing time really an issue? I doubt it, but profile to be sure.

Edit: If anyone is unsure about using single quotes in HTML attributes, have a look at this question. It's pretty unanimously agreed that single quotes are fine. If anyone can give a decent counter-argument I'd be happy to hear it.

edited May 23 '17 at 10:32

Community

1
1

answered Sep 17 '10 at 11:57

Skilldrick

69,215
34
177
229

5

Such bad practice! All attribute values should be enclosed in double quotes as is the standard. – Alex Sep 17 '10 at 12:00
@Alex - If only I could upvote that comment more. We have developers who use single quotes for attribute values, and it makes me lose my mind sometimes. – Joel Etherton Sep 17 '10 at 12:02
thanks @alex and @joel and @skilldrick, so shall i leave my code the way it is. – getaway Sep 17 '10 at 12:06
No, more thorough refactoring is needed ;) why would you want to return that blob of html from a method anyway? – thomasmalt Sep 17 '10 at 12:08
lol @thomasmalt, i dont know it just seemed ideal at the time, basically the function retrieves the users twitter statuses and then echos them in a list hence the "li" list!!! – getaway Sep 17 '10 at 12:12
what I would do, of the bat, is split the code into a template part, containing just the html and then "decorate" the twitter input with the html. – thomasmalt Sep 17 '10 at 12:18
3

@Alex, @Joel, other upvoters: what’s wrong with single quotes? The standard explicitly allows both. Why is it bad practice? – Konrad Rudolph Sep 17 '10 at 12:18
confused.com, is this way standard or not, it looks pretty neat to me – getaway Sep 17 '10 at 12:26
I wouldn't say there's anything wrong exactly, but it's just one of those things that should be a consistency for everyone. And as far as my time in the web design industry goes I've never come across any reason to deviate from that standard. It's like using a , or even tag. They're there, but using them is somehow... wrong. – Alex Sep 17 '10 at 12:52
@Alex You should be pragmatic - it's easier to write PHP with double-quoted strings a single-quoted attributes. For a single line heredoc is too much overhead. There's nothing wrong with single-quoted attributes. Nothing whatsoever. – Skilldrick Sep 17 '10 at 13:58
@Alex - have a read of this: http://stackoverflow.com/questions/273354/html-single-quotes-a-problem – Skilldrick Sep 17 '10 at 14:02
Can't you just use ""; ? – Dai Sep 17 '10 at 14:16
@Dai of course you can, but I find it easier to read 'something quoted' than \"something quoted\". – Skilldrick Sep 17 '10 at 14:17
@Skilldrick - At the end of the day it's down to personal preference. I personally prefer 'something '.$var.' something else' simply because, in my editor, it highlights PHP and fades out HTML allowing me to easily debug/see the PHP snippits mixed in with HTML. However, I'm much more a fan of frameworks over inline PHP anyways – Alex Sep 17 '10 at 14:25
@Alex Personal preference is fine - "Such bad practice" isn't, when it's not. I'm just doing my best to dispel the FUD about single-quoted HTML attributes. – Skilldrick Sep 17 '10 at 14:29
@Skilldrick - I am not against the single quoted stuff, I was just pointing out to all the "double quotes or gtfo" people that you can have the best of both worlds here. – Dai Sep 17 '10 at 15:25
See my question on that: http://stackoverflow.com/questions/2887714/quoting-html-attribute-values – Flavius Sep 17 '10 at 20:12

score 2 · Answer 2 · answered Sep 17 '10 at 11:58

2

Cleaner template and php code -> use MVC

answered Sep 17 '10 at 11:58

Anpher

4,567
24
24

i dnt like the MVC approach i just dnt get it!! thanky anway :)) – getaway Sep 17 '10 at 12:04
1

@getaway Well you'd do a lot better if you pushed yourself abit more with MVC. It is a major 'gateway' to better write code especially with the problem you are facing. – zaf Sep 17 '10 at 12:38
@zaf i tried using codeigntier, i understood the concept, but it was more confusing to me than just normal coding, im not really having trouble with this code, i was just curious if thier was a better way to write it!! – getaway Sep 17 '10 at 12:41
@zaf MVC is architecture. It has nothing to do with this code snipped – Your Common Sense Sep 17 '10 at 14:35
@Col. Shrapnel "MVC is architecture" "It has nothing to do with this code snipped (sic)" Q.E.D. – zaf Sep 17 '10 at 18:31

score 2 · Accepted Answer · answered Sep 17 '10 at 12:14

2

You could use HEREDOC syntax :

$auto = autolink($tweet);
$rel = relativeTime($dt);
return <<<ENDOFRETURN
    <li>
    <a href="nano.com/$username"><img class="avatar" src="images/$picture" width="48" height="48" alt="avatar" /></a>
    <div class="tweetTxt">
    <strong><a href="nano.com/$username">$username</a></strong> $auto
    <div class="date">$rel</div><div class="date">$reply_info</div> <a class ="reply"  href="home.php?replyto=@$username&status_id=$id&reply_name=$username"> reply </a>
    </div>
    <div class="clear"></div>
    </li>
ENDOFRETURN;

answered Sep 17 '10 at 12:14

Toto

89,455
62
89
125

is it not standard to use heredoc syntax – getaway Sep 17 '10 at 12:25
I must admit, I quite like this approach. Tend to keep it for such things as E-Mail templates though. But it'd work just fine. – Alex Sep 17 '10 at 12:53
would i be able to do if statements inside the heredoc – getaway Sep 17 '10 at 13:01
No, you can't, it is just a way to build a string. You can't neither call a function, that's why I assign 2 variables before. – Toto Sep 17 '10 at 13:10
kool kool, its actually much better that way, so you can do all the processing then neatly echo it which is kool, thanks anyway!! – getaway Sep 17 '10 at 13:13
@getaway this ugly syntax won't let your editor to highlight HTML code. If you have idea of code highlighting, of course :) – Your Common Sense Sep 17 '10 at 14:11
@col do you suggest another idea? – getaway Sep 17 '10 at 14:27
@getaway Flavius already suggested down here. If you *really* need to return this code, not output it as it ought to be, you can use ob_start() output buffering to grab PHP code output into a variable – Your Common Sense Sep 17 '10 at 14:33

score 1 · Answer 4 · edited Sep 17 '10 at 14:17

Yes, there is one, and you don't need MVC (only a template):

<li>
 <a href="nano.com/<?=$username ?>">
  <img class="avatar" src="images/<?=$picture ?>" width="48" height="48" alt="avatar" />
 </a>
 <div class="tweetTxt">
  <strong><a href="nano.com/<?=$username ?>"><?=$username ?></a></strong>
  <? echo autolink($tweet) ?>
  <div class="date"><?=relativeTime($dt) ?></div>
  <div class="date"><?=$reply_info ?></div>
  <a class="reply" href="home.php?replyto=@<?=$username?>&status_id=<?=$id?>&reply_name=<?=$username?>">
  reply
  </a>
 </div>
 <div class="clear"></div>
</li>

Must read: http://wiki.yet-another-project.com/php/the_one_single_step_for_a_cleaner_code . It describes how you have to use the code above.

Formatted it a bit. People are way underestimate code formatting/highlighting. — Your Common Sense, Sep 17 '10 at 14:18

score 0 · Answer 5 · answered Sep 17 '10 at 12:00

0

I would cut in several pieces and use sprintf() to tie it all together.

answered Sep 17 '10 at 12:00

Dennis Haarbrink

3,738
1
27
54

Why the downvote? It's a viable solution that could make the intent more clear. – Dennis Haarbrink Sep 17 '10 at 12:18
+1 : This is one reasonable way of cleaning it up. Don't hold your breath for the down voter to explain why this is not useful. – zaf Sep 17 '10 at 12:33
@zaf well, if you insists. 1. no code example to estimate the real value of such an improvement. sprintf is still ugly approach to encapsulate HTML into PHP into string into function... Leave HTML alone! It's a language too. And it deserves formatting/highlighting. And a programmer, who would read this code, would save some headache for more important task. – Your Common Sense Sep 17 '10 at 14:31
Of course there are better ways than this. To correctly solve the problem his design needs to be reconsidered IMHO. My suggested approach might not fit *your* ideal of good code, and I would agree with you, but it could definitely solve/improve this problem in isolation. – Dennis Haarbrink Sep 17 '10 at 14:44
@Col. Shrapnel Thank you for returning and leaving a comment! Upvote for you but I'll also keep the upvote to this question. – zaf Sep 17 '10 at 18:26

score 0 · Answer 6 · answered Sep 17 '10 at 12:25

0

You can use a template engine i.e. smarty, twig, ...

answered Sep 17 '10 at 12:25

ipsum

1,042
7
17

Is there a better way to write this php code?

6 Answers6

Linked