Function inside of if statement?

JustCoding

So, I have this basic code. It pretty much checks a user input to see if it is a Youtube URL. If it is, then it grabs the Youtube ID. After getting the Youtube ID it then checks to see if the video is still active.

The problem is with the current code, I am either getting one or two outputs:

The link provided is not a working Youtube link. Please try again.

The video is currently active!

I believe the problem is when trying to use function checkYoutubeOnline and then checking the value. Does anyone have any helpful advice or would like to give feedback or tips? I appreciate the time to look over my code! Thanks a bunch.

       $youtubeid = youtube_id_from_url($link); //Gets Youtube ID from URL.

if ($youtubeid != false){ //If was able to retrieve ID continue
checkYoutubeOnline($youtubeid); //Check to see if Youtube link is active
if ($youtubeid = true){ //Check result from function
echo 'The video is currently active!';
}else{
echo 'This video is no longer active.';
}
	}
}else{
	echo 'The link provided is not a working Youtube link. Please try again.';
}

The functions are

function youtube_id_from_url($url) {
    $pattern = 
        '%^# Match any youtube URL
        (?:https?://)?  # Optional scheme. Either http or https
        (?:www\.)?      # Optional www subdomain
        (?:             # Group host alternatives
          youtu\.be/    # Either youtu.be,
        | youtube\.com  # or youtube.com
          (?:           # Group path alternatives
            /embed/     # Either /embed/
          | /v/         # or /v/
          | /watch\?v=  # or /watch\?v=
          )             # End path alternatives.
        )               # End host alternatives.
        ([\w-]{10,12})  # Allow 10-12 for 11 char youtube id.
        $%x'
        ;
    $result = preg_match($pattern, $url, $matches);
    if ($result) {
        return $matches[1];
    }else{
    return false;
}
}

function checkYoutubeOnline($youtubeid) {  

if (!$data = @file_get_contents("http://gdata.youtube.com/feeds/api/videos/".$youtubeid)) return false;
if ($data == "Video not found") return false;
return true;
}

bradgrafelman

Two problems here:

    checkYoutubeOnline($youtubeid); //Check to see if Youtube link is active 
    if ($youtubeid = true){ //Check result from function

First and foremost, you never store or directly examine the value [man]return[/man]ed by the checkYoutubeOnline() function. Since that function doesn't really do anything but return a value, you might as well not even be calling the function at all.
Your if() statement is assigning the value true to the variable $youtubeid. If you wanted to compare values, then you need to use a comparison operator (see [man]operators.comparison[/man]).

Weedpacket

And 3. (which actually combines 1. and 2.) even if you were comparing values, you're not comparing the right values.

Bonesnap

Regarding point #2 raised by Brad, I find it good practice to put the constant on the left hand side of comparisons that way you are met with an error immediately and can correct it, otherwise you run the risk of having code like above where it appears to be working but actually isn't, and then you have to spend time going through everything wondering where the error is.

laserlight

Bonesnap wrote:
I find it good practice to put the constant on the left hand side of comparisons that way you are met with an error immediately and can correct it, otherwise you run the risk of having code like above where it appears to be working but actually isn't, and then you have to spend time going through everything wondering where the error is.

I do not find it a good practice to do that because it is not always the case that the left hand side of comparisons is a constant, and I find it irritating since it reverses the canonical way of reading such expressions where a constant is involved. Rather, the good practices here involve things like automated testing, pair programming, code review, and usage of static analysis tools that can spot such potential mistakes.

Bonesnap

Well the order of the conditions in an if statement is arbitrary and you can do whatever you want. Not all conditions will have a constant but I find enough of them do. But if you do have a constant then placing it on the left avoids these situations. It does take some getting used to when reading them and sometimes I find myself forgetting and writing it the other way, but when I catch myself I correct it. The good practices you listed are all great but not all workplaces have them implemented or available. My workplace doesn't have any of them; we do thorough QA on our websites to ensure as few bugs make it to delivery as possible, but that's it. Something like this would identify the bug immediately and wouldn't even make it to QA since it would already be fixed.