Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Route::clone #169

Closed
bkdotcom opened this issue Dec 18, 2019 · 4 comments
Closed

Route::clone #169

bkdotcom opened this issue Dec 18, 2019 · 4 comments

Comments

@bkdotcom
Copy link
Contributor

bkdotcom commented Dec 18, 2019

Route::__clone

    /**
     *
     * When cloning the Route, reset the `$attributes` to an empty array, and
     * clear the `$failedRule`.
     *
     */
    public function __clone()
    {
        // $this is the cloned instance, not the original
        $this->attributes = $this->defaults;
        $this->failedRule = null;
    }

comment says one thing / code does another

$map->allows(['GET', 'HEAD', 'POST'])->defaults(array(
    'action' => 'foo',
));
$route = $map->route('test', '/test');
$route->defaults(array('action'=>'not foo'));
var_dump($route->attributes);   // array('action'=>'foo')...  I would expect empty array()
var_dump($route->defaults);     // array('action'=>'not foo')

when dealing with routes I don't know if attribute came from url, or some inherited default
If I use Generator to build this new route it's going to use the inherited default over the new default

$this->attributes = $this->defaults;    // this does NOT reset `$attributes` to an empty array!!
@bkdotcom
Copy link
Contributor Author

bkdotcom commented Dec 18, 2019

updating Route::__clone && Route::__get to the following seems to do the trick for me

    /**
     * When cloning the Route, reset the `$attributes` to an empty array, and
     * clear the `$failedRule`.
     */
    public function __clone()
    {
        // $this is the cloned instance, not the original
        $this->attributes = array();
        $this->failedRule = null;
    }

    /**
     * Magic read-only for all properties.
     *
     * @param string $key The property to read from.
     *
     * @return mixed
     */
    public function __get($key)
    {
        if ($key === 'attributes') {
            return array_merge($this->defaults, $this->attributes);
        }
        return $this->$key;
    }

@harikt
Copy link
Member

harikt commented Jan 26, 2022

I agree the comment When cloning the Route, reset the $attributes to an empty array was a typo. It probably should have mentioned, will use defaults if any or empty start array. Apart from that the code seems working as expected to me.

Next time if possible send a failing test case. It may be helpful.

harikt added a commit that referenced this issue Feb 1, 2022
@harikt
Copy link
Member

harikt commented Feb 1, 2022

Hi @bkdotcom ,

I know this is an old issue. But just wanted to know if updating doc block as in #190 will remove all the confusions or not.

I will be releasing a 3.2.0 today. In case you are not satisfied with the resolution please do open it.

Thank you for the report.

@harikt harikt closed this as completed Feb 1, 2022
harikt added a commit that referenced this issue Feb 1, 2022
@bkdotcom
Copy link
Contributor Author

bkdotcom commented Feb 1, 2022

@harikt
it does!
Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants