SitePoint Sponsor

User Tag List

Results 1 to 12 of 12
  1. #1
    SitePoint Guru
    Join Date
    Jan 2007
    Posts
    934
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)

    Please critique this code

    Below is a script that extracts information from embed codes for Vimeo and Youtube. It returns a thumbnail, video with customized skin, or the movie ID.

    I'm new to OOP and I'm unclear on storing and returning the results.
    At the end of each function it has something like:
    Code PHP:
    $this->Id=$id;
    return $id;

    It seems redundant, but I want the function to work Independently as well as be able to work with other functions.

    Also is there a more elegant way to extract the id from the Youtube Embed code?

    Any other feedback is also welcome.

    Feel free to use this if it is helpful to you.

    Thank You E.

  2. #2
    SitePoint Guru
    Join Date
    Jan 2007
    Posts
    934
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    The Code

    Code PHP:
    <?php
     
    /* usage
     
    require('embed_code_extraction.php');
    $embed_extraction = new EmbedCodeExtraction();
    $embed_extraction->EmbedCode=$media['video'];
    $embed_extraction->VideoHeight=250;
    $embed_extraction->VideoWidth=350;
    $thumbnail=$embed_extraction->get_embed_thumbnail();
    $video_id=$embed_extraction->get_embed_id();
    $video=$embed_extraction->get_embed_video()
     
    */
     
    class EmbedCodeExtraction{
     
    	public $EmbedCode;
    	public $Id;
    	public $Thumbnail;
    	public $Video;
    	public $EmbedOutput;
    	public $EmbedType;	
    	public $VideoHeight=250;
    	public $VideoWidth=350;
     
    	function get_embed_id(){
     
    		$type=$this->get_embed_type();
     
    		if($type=='vimeo'){
    			$id=$this->find_vimeo_id();
    		}
     
    		else if( $type=='youtube'){
    			$id=$this->find_youtube_id();
    		}
     
    		$this->Id=$id;
    		return $id;
     
    	}
     
    	function get_embed_thumbnail(){
     
    		$type=$this->get_embed_type();
     
    		if($type=='vimeo'){
    			$id=$this->find_vimeo_id();
    			$thumbnail=$this->get_vimeo_thumbnail();
    		}
     
    		else if( $type=='youtube'){
    			$id=$this->find_youtube_id();
    			$thumbnail=$this->get_youtube_thumbnail();
    		}
     
    		$this->Thumbnail=$thumbnail;
    		return $thumbnail;
     
    	}
     
     
    	function get_embed_video(){
     
    		$type=$this->get_embed_type();
     
    		if($type=='vimeo'){
    			$Id=$this->find_vimeo_id();
    			$video=$this->write_vimeo_video();
    		}
     
    		else if( $type=='youtube'){
    			$Id=$this->find_youtube_id();
    			$video=$this->write_youtube_video();
    		}
     
    		return $video;
     
    	}
     
    	//identify YouTube or Vimeo
    	function get_embed_type(){
     
    		$embed=$this->EmbedCode;
     
    		if(strpos($embed,'vimeo')>0 ){ $type='vimeo'; }
    		else if (strpos($embed,'youtube')>0 ){ $type='youtube'; }
    		else { $type='unknown'; }
     
    		$this->EmbedType=$type;
    		return $type;
     
    	}
     
    	/* VIMEO EXTRACTION */
    	function get_vimeo_thumbnail(){
     
    		$id=$this->find_vimeo_id($embed);
     
    		if($id){
    			$hash = unserialize(file_get_contents("http://vimeo.com/api/v2/video/$id.php"));
    			$thumbnail= $hash[0]['thumbnail_small'];
    			$this->Thumbnail=$thumbnail;
    			return $thumbnail;
    		}
     
    	}
     
    	function write_vimeo_video(){
     
    		$id=$this->find_vimeo_id();
    		$h=	$this-> VideoHeight;
    		$w= $this-> VideoWidth;
     
    		if($id){
    			$movie='<iframe src="http://player.vimeo.com/video/'.$id.'?title=0&amp;byline=0&amp;portrait=0&amp;color=E68C35" width="'.$w.'" height="'.$h.'" frameborder="0"></iframe>';
     
    			return $movie;
    		}
     
    	}
     
     
    	function find_vimeo_id(){
    			$embed=$this->EmbedCode;
     
    		if(strpos($embed,'vimeo')>0 && strpos($embed,'clip_id=')>0 ){
    			$start=strpos($embed,'clip_id=')+8;
    			$end=strpos($embed,'&',$start);
    			$stop=$end-$start;
    			$id=substr( $embed, $start,$stop);
    		}
    		else if(strpos($embed,'vimeo')>0 && strpos($embed,'http://player.vimeo.com/video/')>0){
    			$start=strpos($embed,'/video/')+7;
    			$end=strpos($embed,'?',$start);
    			$stop=$end-$start;
    			$id=substr( $embed, $start,$stop);
    		}
     
     
    		$this->Id=$id;
    		return $id;
     
    	}
     
    	/* END VIMEO EXTRACTION */
     
    	/* YOUTUBE EXTRACTION */
     
    	function get_youtube_thumbnail(){
     
    		$id=$this->Id;
     
    		if($id){
    			$thumbnail= "http://i2.ytimg.com/vi/$id/default.jpg";
    			$this->Thumbnail=$thumbnail;
    			return $thumbnail;
    		}
     
    	}
     
    	function write_youtube_video(){
     
    		$id=$this->find_youtube_id();
    		$h=	$this-> VideoHeight;
    		$w= $this-> VideoWidth;
     
    		$video='
    		<object width= "'.$w.'" height="'.$h.'">
     
    			<param name="movie" value="http://www.youtube.com/v/'.$id.'&autoplay=0&rel=0&fs=1&color1=0x4E3400&color2=0x322A22&border=0&loop=0"></param>
     
    			<param name="allowFullScreen" value="true"></param>
     
    			<embed src="http://www.youtube.com/v/'.$id.'&autoplay=0&rel=0&fs=1&color1=0x4E3400&color2=0x322A22&border=0&loop=0" type="application/x-shockwave-flash" allowfullscreen="true" width="'.$w.'" height="'.$h.'"></embed>
     
    		</object>
    		';
     
    		$this->Video=$video;
    		return $video;
     
    	}
     
    	/* 
     
    	EXAMPLE YOUTUBE VARIATIONS 
     
    	[url=http://www.youtube.com/watch?v=FQDgVkasz7o]YouTube - &#x202a;Bolsa de Bota Atelier Favela&#x202c;&rlm;[/url]
     
    	<object width="425" height="344"><embed src="http://www.youtube.com/v/4Dq8dwyV35k?hl=en&fs=1" type="application/x-shockwave-flash" allowscriptaccess="always" allowfullscreen="true" width="425" height="344"></embed></object>
     
    	<object width=""425"" height=""344"" data=""http://www.youtube.com/v/wPsru6TlgfI%26hl=en%26fs=1%26rel=0%26ap=%2526fmt=18"" type=""application/x-shockwave-flash"">
    	<param name=""src"" value=""http://www.youtube.com/v/wPsru6TlgfI%26hl=en%26fs=1%26rel=0%26ap=%2526fmt=18"" />
    	</object>
     
    	<iframe title="YouTube video player" width="640" height="390" src="http://www.youtube.com/embed/VvC5HfUqa28" frameborder="0" allowfullscreen></iframe>
     
    	<iframe title="YouTube video player" width="640" height="390" src="http://www.youtube.com/embed/taFNdDs1SVU?rel=0" frameborder="0" allowfullscreen></iframe>
     
    	*/
     
    	function find_youtube_id(){
     
    		$embed=$this->EmbedCode;
     
    		if(strpos($embed,'watch?v=')>0 ){
    			$start=strpos($embed,'watch?v=')+8;
    			$id=substr( $embed, $start);
    		}
    		else if(strpos($embed,'youtube.com/v/')>0){
    			$start=strpos($embed,'/v/')+3;
    			$end=strpos($embed,'?',$start);
    			if(strpos($embed,'%')>0 && !$end){
    				$end=strpos($embed,'%',$start);
    			}
    			else if(strpos($embed,'"' )>0 && !$end){
    				$end=strpos($embed,'"',$start);
     
    			}
    			else if(strpos($embed,"'")>0 && !$end) {
    				$end=strpos($embed,'\'',$start);
    			}
    			$stop=$end-$start;
    			$id=substr( $embed, $start,$stop);
    		}
     
    		else if(strpos($embed,'/embed/')>0){
     
    			$start=strpos($embed,'/embed/')+7;
    			$end=strpos($embed,'?',$start);
     
    			if(strpos($embed,'%')>0 && !$end){
    				$end=strpos($embed,'%',$start);
    			}
    			else if(strpos($embed,'"' )>0 && !$end){
    				$end=strpos($embed,'"',$start);
     
    			}
    			else if(strpos($embed,"'")>0 && !$end) {
    				$end=strpos($embed,'\'',$start);
    			}
    			$stop=$end-$start;
    			$id=substr( $embed, $start,$stop);
    		}
     
    		$id=trim($id);
     
    		$this->Id=$id;
    		return $id;
     
    	}
     
     
    	/* END YOUTUBE EXTRACTION */
     
    } //end class
     
    ?>

  3. #3
    SitePoint Addict
    Join Date
    Oct 2003
    Location
    United States
    Posts
    281
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I didn't look through all of it, but You could easily rewrite this:

    PHP Code:
        function get_embed_id(){
           
            
    $type=$this->get_embed_type();
           
            if(
    $type=='vimeo'){
                
    $id=$this->find_vimeo_id();
            }
           
            else if( 
    $type=='youtube'){
                
    $id=$this->find_youtube_id();
            }
                   
            
    $this->Id=$id;
            return 
    $id;
           
        } 
    Into:

    PHP Code:
        function get_embed_id(){
           
            
    $type $this->get_embed_type();
           
            if(
    $type == 'vimeo'){
                
    $this->Id $this->find_vimeo_id();
            }
           
            else if( 
    $type == 'youtube'){
                
    $this->Id $this->find_youtube_id();
            }
                   
            return 
    $this->Id;
           
        } 
    Or even something like...

    PHP Code:
        function get_embed_id(){
           
            
    /* Creates find_vimeo_id or find_youtube_id */
            
    $method 'find_' $this->get_embed_type() . '_id';
            
            
    // If this function exists.. run it
               
    if( method_exists($this$method)) {
              
    $this->Id call_user_func(array($this$method));
            }

            return 
    $this->Id;
           
        } 

  4. #4
    SitePoint Guru
    Join Date
    Jan 2007
    Posts
    934
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    Combining the two statements makes a lot of sense.
    return $this->Id;

    Your second variation is interesting to see, but I think it obscures the logic and doesn't seem like it reduces the code much.

    Thank You E!

  5. #5
    Utopia, Inc. silver trophy
    ScallioXTX's Avatar
    Join Date
    Aug 2008
    Location
    The Netherlands
    Posts
    8,892
    Mentioned
    138 Post(s)
    Tagged
    2 Thread(s)
    I have a few questions:

    1) Why are all properties of the class public?
    2) Why didn't you make it a normal class (as opposed to using the singleton pattern)?

    And I don't want to a party pooper here, but is there any reason you didn't use oEmbed ? Makes life a lot easier
    Rémon - Hosting Advisor

    Minimal Bookmarks Tree
    My Google Chrome extension: browsing bookmarks made easy

  6. #6
    SitePoint Guru
    Join Date
    Jan 2007
    Posts
    934
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by ScallioXTX View Post
    I have a few questions:

    1) Why are all properties of the class public?
    2) Why didn't you make it a normal class (as opposed to using the singleton pattern)?
    Looks like I could make these properties private because they are only defined by the script. correct?

    Code PHP:
        public $Id;
        public $Thumbnail;
        public $Video;
        public $EmbedOutput;
        public $EmbedType;

    What's a Singleton Pattern? I googled it but didn't understand the results.
    Is there a simple way to describe your comment?

    Many thanks E.

  7. #7
    Utopia, Inc. silver trophy
    ScallioXTX's Avatar
    Join Date
    Aug 2008
    Location
    The Netherlands
    Posts
    8,892
    Mentioned
    138 Post(s)
    Tagged
    2 Thread(s)
    Your usage example right now is :

    PHP Code:
    require('embed_code_extraction.php');
    $embed_extraction = new EmbedCodeExtraction();
    $embed_extraction->EmbedCode=$media['video'];
    $embed_extraction->VideoHeight=250;
    $embed_extraction->VideoWidth=350;
    $thumbnail=$embed_extraction->get_embed_thumbnail();
    $video_id=$embed_extraction->get_embed_id();
    $video=$embed_extraction->get_embed_video() 
    With the singleton pattern it would become:

    PHP Code:
    require('embed_code_extraction.php');
    $thumbnail=EmbedCodeExtraction::get_embed_thumbnail($media['video']);
    $video=$embed_extraction->get_embed_video($media['video'], 350250); 
    Of course you would then have to change the code to be able to work like this, but from what I can see this way is a lot cleaner because the way you've set up now you're meddling with the internals of a class from outside the class, and that's never a good thing.

    Another way to go about it would be to say that you will only ever have one video object at a time, and if there are multiple videos at one page you'll just use that same object over and over again but you'll never have multiple instances of that one class at any one time, which is a strong indicator that you might use the singleton pattern instead.

    It's kind of a tough subject and I'm not sure if I'm explaining this well. If not please let me know.

    Also, have you looked at oEmbed?
    Rémon - Hosting Advisor

    Minimal Bookmarks Tree
    My Google Chrome extension: browsing bookmarks made easy

  8. #8
    SitePoint Guru
    Join Date
    Jan 2007
    Posts
    934
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    Thanks for explaining this. oEmbed looks good. Though, what I wrote is working well.

    E

  9. #9
    Utopia, Inc. silver trophy
    ScallioXTX's Avatar
    Join Date
    Aug 2008
    Location
    The Netherlands
    Posts
    8,892
    Mentioned
    138 Post(s)
    Tagged
    2 Thread(s)
    Yes apart from the structure I don't really agree with, the functions themselves look good. Very neat and lean, I like it. For now I'd indeed go with this, but it might be an idea to switch to oEmbed if you ever need to support more providers than just youtube and vimeo
    Rémon - Hosting Advisor

    Minimal Bookmarks Tree
    My Google Chrome extension: browsing bookmarks made easy

  10. #10
    SitePoint Guru
    Join Date
    Jan 2007
    Posts
    934
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    Thank you Skallio for your feedback. I'll have to do more reading to really understand what you are saying. I'll keep oEmbed that in mind. For now those are the only two formats that have been used on the site.

    E

  11. #11
    SitePoint Wizard silver trophy kyberfabrikken's Avatar
    Join Date
    Jun 2004
    Location
    Copenhagen, Denmark
    Posts
    6,157
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Why are you assigning the values to a property at all? Since it's calculated when calling the method (function), there is no reason to store it.

  12. #12
    SitePoint Guru
    Join Date
    Jan 2007
    Posts
    934
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    That's a good point.

    These variables could just be returned and not stored:

    public $Thumbnail;
    public $Video;
    public $EmbedOutput;

    Whereas these ones are reused:

    private $Id;
    private $EmbedType;

    public $EmbedCode;
    public $VideoHeight=250;
    public $VideoWidth=350;


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •