Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm trying to create URL parse class., so i can check URL Segment / part,

Example:

  1. http://localhost/myproject/class/method/arg/other (localhost)
  2. http://www.myproject.com/class/method/arg/other (live site)
  3. http://www.myproject.com/class/method/arg/other?param=value (with get param)

In urls above:

  • $url->segment(1) : class
  • $url->segment(2) : method

If argument null, it will return the last segment:

  • $url->segment() : other

I need suggestions for improvement this class, here is the code:

<?php
class url
{
    private $url;

    public function segment($arg=null)
    {
        if($arg==null)
        {
            $this->url = str_replace(BASEDIR,'',$_SERVER['REQUEST_URI']);

            if(isset($_GET))
            {
                $this->url = explode('?',$this->url);
                $this->url = $this->url[0];
            }

            $this->url = explode('/', trim($this->url, '/'));       


            return end($this->url);
        }
        else
        {
            $this->url = str_replace(BASEDIR,'',$_SERVER['REQUEST_URI']);
            if(isset($_GET))
            {
                $this->url = explode('?',$this->url);
                $this->url = $this->url[0];
            }

            $this->url = explode('/', trim($this->url, '/'));

            array_unshift($this->url, null);

            unset($this->url[0]);

            if(isset($this->url[$arg]))
            {
                return $this->url[$arg];
            }
            else
            {
                return null;
            }
        }
    }
}    
?>
share|improve this question

1 Answer 1

up vote 1 down vote accepted

Just a quick note: there is some duplication in the two branches of the first if. You could write the following:

<?php
    public function segment($arg=null)
    {
        $this->url = str_replace(BASEDIR,'',$_SERVER['REQUEST_URI']);
        if(isset($_GET))
        {
            $this->url = explode('?',$this->url);
            $this->url = $this->url[0];
        }
        $this->url = explode('/', trim($this->url, '/'));       


        if($arg==null)
        {
            return end($this->url);
        }
        else
        {
            array_unshift($this->url, null);
            unset($this->url[0]);
            if(isset($this->url[$arg]))
            {
                return $this->url[$arg];
            }
            else
            {
                return null;
            }
        }
    }
}    
?>

Actually, the else keyword is unnecessary since if the $arg == null condition is true the function returns.

    ...
    if($arg==null)
    {
        return end($this->url);
    }

    array_unshift($this->url, null);
    unset($this->url[0]);
    if(isset($this->url[$arg]))
    {
        return $this->url[$arg];
    }
    // else is unnecessary here too
    return null;
share|improve this answer

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.