Structure problem: trying to clean my code with a switch statement...

This site is best viewed in a modern browser with JavaScript enabled.

Structure problem: trying to clean my code with a switch statement...

Ness_du_Frat

Hi!
I'm working on a little thing for my website (modifying an existing wordpress plugin, so that I can style the comments from the authors even if they post on facebook on the comment in the posted on wordpress via the plugin).

My code (working) is awful :

	function Comment_class($classes) {
			global $comment;
			if (!empty($comment) && $comment->comment_agent == 'AL2FB')
				$classes[] = 'facebook-comment';
						if (!empty($comment) && $comment->comment_agent == 'AL2FB' && esc_attr($comment->comment_author_url) == 'http://www.facebook.com/profile.php?id=randomnumber1')
				$classes[] = 'comment-author-ec';
						if (!empty($comment) && $comment->comment_agent == 'AL2FB' && esc_attr($comment->comment_author_url) == 'http://www.facebook.com/profile.php?id=randomenumber2')
				$classes[] = 'comment-author-ness';
						if (!empty($comment) && $comment->comment_agent == 'AL2FB' && esc_attr($comment->comment_author_url) == 'http://www.facebook.com/profile.php?id=randomnumber3')
				$classes[] = 'comment-author-alice';
						if (!empty($comment) && $comment->comment_agent == 'AL2FB' && esc_attr($comment->comment_author_url) == 'http://www.facebook.com/profile.php?id=randomnumber4' || !empty($comment) && $comment->comment_agent == 'AL2FB' && esc_attr($comment->comment_author_url) == 'http://www.facebook.com/profile.php?id=randomnumber5')
				$classes[] = 'comment-author-joannv';
						if (!empty($comment) && $comment->comment_agent == 'AL2FB' && esc_attr($comment->comment_author_url) == 'http://www.facebook.com/profile.php?id=randomnumber6')
				$classes[] = 'comment-author-kanata';
			return $classes;
		}

Here is my attempt :

function Comment_class($classes) {
			global $comment;
						if (!empty($comment) && $comment->comment_agent == 'AL2FB' && esc_attr($comment->comment_author_url) == $urlfacebook)
				$classes[] = $facebookclasshere;
						switch ($urlfacebook) {

			case 'http://www.facebook.com/profile.php?id=randomnumber1':			
			$facebookclasshere = 'comment-author-ec';
			break;

			case 'http://www.facebook.com/profile.php?id=randomnumber2':
			$facebookclasshere = 'comment-author-ness';
			break;

			case 'http://www.facebook.com/profile.php?id=randomnumber3':
			$facebookclasshere = 'comment-author-alice';
			break;

			case 'http://www.facebook.com/profile.php?id=randomnumber4':				
			case 'http://www.facebook.com/profile.php?id=randomnumber5':
			$facebookclasshere = 'comment-author-joannv';
			break;

			case 'http://www.facebook.com/profile.php?id=randomnumber6':
			$facebookclasshere = 'comment-author-kanata';
			break;
			}
		return $classes;
}

No idea why it's not working Somebody could give me a hint? Thanks a lot !

NogDog

Main problem I see is that you try to assign $facebookclasshere to your array before it ($facebookclasshere, that is) gets defined/assigned in the following switch() block. Looks like that assignment should happen after the switch() finishes its work. I suspect this is your show-stopper issue -- the following are probably not crticial, but would add robustness and maintainability:

For more robustness, you may want/need a "default" case to do whatever seems appropriate if you fall through all the other cases without a match (even if it's just logging something to the error_log).

Lastly, "global" should be avoided except as a last resort. It's better to pass it in as another function parameter if possible, passing it by reference if you need to modify it as part of the function.

Ness_du_Frat

Thanks!
Yeah, it's not my function, I'm just adding the switch. The global and all are part of the plugin, so I'd rather not touch it.
I reordered it and put the

							[code=php]$classes[] = $facebookclasshere;

[/code]

after my switch, but it doesn't work either
At the moment, my ugly code is working, but I don't like when it's a mess like that...

NogDog

Upon further review, I think maybe something like this is what you're after?

function Comment_class($classes) {
	global $comment; // Yuk! ;-)
	if (!empty($comment) && $comment->comment_agent == 'AL2FB') {
		switch (esc_attr($comment->comment_author_url)) {
			case 'http://www.facebook.com/profile.php?id=randomnumber1':            

				$facebookclasshere = 'comment-author-ec';
				break;
			case 'http://www.facebook.com/profile.php?id=randomnumber2':
				$facebookclasshere = 'comment-author-ness';
				break;
			case 'http://www.facebook.com/profile.php?id=randomnumber3':
				$facebookclasshere = 'comment-author-alice';
				break;
			case 'http://www.facebook.com/profile.php?id=randomnumber4':                

			case 'http://www.facebook.com/profile.php?id=randomnumber5':
				$facebookclasshere = 'comment-author-joannv';
				break;
			case 'http://www.facebook.com/profile.php?id=randomnumber6':
				$facebookclasshere = 'comment-author-kanata';
				break;
		}
	}
	$classes[] = $facebookclasshere;
	return $classes;
}

Ness_du_Frat

Oh, awesome, perfectly working, thanks a lot!

NogDog

PS: This would be less likely to generate undefined variable warnings:

function Comment_class($classes) {
    global $comment; // Yuk! ;-)
    if (!empty($comment) && $comment->comment_agent == 'AL2FB') {
        switch (esc_attr($comment->comment_author_url)) {
            case 'http://www.facebook.com/profile.php?id=randomnumber1':            

                $classes[] = 'comment-author-ec';
                break;
            case 'http://www.facebook.com/profile.php?id=randomnumber2':
                $classes[] = 'comment-author-ness';
                break;
            case 'http://www.facebook.com/profile.php?id=randomnumber3':
                $classes[] = 'comment-author-alice';
                break;
            case 'http://www.facebook.com/profile.php?id=randomnumber4':                

            case 'http://www.facebook.com/profile.php?id=randomnumber5':
                $classes[] = 'comment-author-joannv';
                break;
            case 'http://www.facebook.com/profile.php?id=randomnumber6':
                $classes[] = 'comment-author-kanata';
                break;
        }
    }
    return $classes;
}

Ness_du_Frat

Thanks!
The other one seems to be working perfectly fine, I added a default case so that the variable is defined anyway.
Why would it generate undefined variables warnings?
(I'm sorry, now I'm working with php like once a year, so I forgot a lot about it...)

NogDog

The prior version only set $facebookclasshere within the if() block, but then used it after that if() block. Therefore, if at any time the if() condition did not trigger, after that block you would then be trying to use $facebookclasshere in an undefined state.

Ness_du_Frat

Ok, thanks for the explanation!