init var inside if() ?

[Xdega]

I just want to know if this line of code is as messy as I am thinking. I think is is kinda odd from a logic perspective and I have never seen anything like it:

 

<?php if ( $company_url = esc_url( get_post_meta( $post->ID, '_CompanyURL', true ) ) ) { //stuff here } ?>

 

What are your thoughts on this?

While I don't think it is critical to the functionality, what kind of revisions could/would/should be made to the above line of code?

[requinix]

Matter of style. My opinion:

 

If you set variables inside a condition (like an if or while) then the variable is only good inside the subsequent code block. Picked up that practice with languages that have stricter scope rules (like C). If you need the variable later then initialize it before the condition.

if ($var = get_var()) {
    // use $var
}
// pretend $var doesn't exist

$var = get_var();
if ($var) {
    // use $var
}
// can still use $var

 

Other thing: allowing yourself that syntax increases the odds that you'll accidentally use a = when you want a ==. You'd have to pay closer attention to the code to decide whether that typo is intentional.

[Xdega]

I c... I also discussed this with a friend of mine. I guess you raise some pretty clear points.

It is pretty clear what they are trying to accomplish, I guess it makes perfect sense. (although my IDE hates it)

I think it just appears counter-intuitive that there is an assignment, instead of a clear condition/comparison inside an IF

 

It seems like potentially a way to ensure that the variable is being correctly initialized?

I would assume that it would return false if any of the function calls fail?

In which case an else {echo "error!" ;} could be used?

[kicken]

I would assume that it would return false if any of the function calls fail?

 

Not if any of the function calls fail, just if the outer most one failed (or rather, returns a false-like value).  In your given example the only thing that matters as far as the if condition is concerned is what is returned from the esc_url function.  Most likely the way it works means that if get_post_meta returned a false value, so would esc_url but that may not always be the case.

 

//---

 

I will initialize variables inside a condition like that sometimes, but I try to keep the number of operations to a minimum in the if.  Based on the function names in your example, I am guessing that esc_url is basically a urlencode type function.  If that's the case, I would probably write it like this:

 

if ($company_url = get_post_meta( $post->ID, '_CompanyURL', true )) {
    $company_url = esc_url($company_url); 
    //more stuff
}

 

Reasons being that get_post_meta is the function that might fail, and esc_url is just a simple encoding process that won't fail.  It keeps the code a bit easier to read on that if statement line by lowering the number of () and having less stuff going on.

 

 

If on the other hand esc_url could actually fail, I would still split it out, but put the get_post_meta on the line above and esc_url in the if statement.

 

Ultimately it is just a personal style choice whether you want to do it that way or not though, just make sure the code makes sense and is easy to read/understand.

 

[smoseley]

Yes, it's a little messy messy. 

 

It's a best practice to split separate operations onto separate lines of code, e.g.:

 

$company_url = get_post_meta($post->ID, '_CompanyURL', true);
if (!is_null($company_url)) {
    $company_url = esc_url($company_url); 
    //more stuff
}