Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I want to find out if there are any improvements I can make to this code. How can I make it better, faster and use less queries? Or is it fine the way I wrote it?

<?php if ( $user_ID ) : ?>
<?php
global $user_identity;
?>
<div id="boxtop">
<div class="boxtop">
<div class="bus">
<div class="busl">
<span class="bin">
Bine ai revenit<a href="/edit-profile/"><b><?php echo $user_identity; ?></b> !</a>
</span>
<?php 
global $current_user;
$current_user = wp_get_current_user();
?>

<a href="/utilizatori-inregstrati/">Utilizatori</a>|<a href="/edit-profile/">Profil</a>|<a href="<?php echo wp_logout_url( get_bloginfo('url') ); ?>" title="Logout">Logout</a>
</div>
<div class="busr">
<?php 
$current_user = wp_get_current_user();
if ( ($current_user instanceof WP_User) ) {
    echo get_avatar( $current_user->user_email, 40 );
}
?>
</div>
</div>
 </div>
</div>
<?php else : ?>

<div id="boxtop">
<div class="boxtop">
<a class="fblogin" href="http://www.mysite.ro/wp-login.php?loginFacebook=1&redirect=http://www.mysite.ro" onclick="window.location = 'http://www.mysite.ro/wp-login.php?loginFacebook=1&redirect='+window.location.href; return false;"><img src="/wp-content/themes/movies/images/fb.png" title="Login with Facebook"  /></a>
<form name="loginform" id="loginform" action="<?php echo get_option('siteurl'); ?>/wp-login.php" method="post">
<input value="Utilizator" class="input" type="text" size="10" tabindex="10" name="log" id="user_login" onfocus="if (this.value == 'Utilizator') {this.value = '';}" onblur="if (this.value == '') {this.value = 'Utilizator';}" />
<input value="Parola" class="input" type="password" size="10" tabindex="20" name="pwd" id="user_pass" onfocus="if (this.value == 'Parola') {this.value = '';}" onblur="if (this.value == '') {this.value = 'Parola';}" />
<input name="wp-submit" id="wp-submit" value="Login" tabindex="100" type="submit">
<span class="bin">
<span><a href="/recupereaza-parola/">Recuperare parola</a></span><span><a href="/inregistrare/">Inregistrare</a></span><input name="rememberme" id="rememberme" value="forever" tabindex="90" type="checkbox"> <span>Retine parola?</span>
</span>
<input name="redirect_to" value="<?php echo get_option('siteurl'); ?>" type="hidden">
<input name="testcookie" value="1" type="hidden">
</form>
    </div>
</div>
</div>
<?php endif; ?>
share|improve this question
add comment

1 Answer 1

Ok, a few things of note. Not really related to perfomance, but readability. To be fair though these are just details for better readability, your code is not so bad.

This code is not expensive process wise, I was curious as to where $User_id comes from, because that is where the query you are worried about comes into play, but I trust your tables are properly indexed and output data is properly escaped.

On line 12-14, I would put that php code up at top, all in a single php block. This would separate the php from the html and make for better readability.

Also, same thing for the php block on line 21. You call once again $current_user = wp_get_current_user(); , but you already have $current_user from line 15.

I would take this block to the top, and change the echo get_avatar() call to :

$avatar = "";
if ( ($current_user instanceof WP_User) ) {
  $avatar =  get_avatar( $current_user->user_email, 40 );
} 

By initialising the variable (best practice), in your html "template code" below you can get away with just:

if ($avatar) { print $avatar; }

This habit leaves minimal php in the html and thus separates the logic from presentation.

I assume indentation was lost when you pasted, so I will not patronize you on this.

Hope this helps, happy coding.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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