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

[8.x] Added whenEndsWith(), whenExactly(), whenStartsWith(), etc to Stringable #40320

Merged
merged 5 commits into from
Jan 11, 2022

Conversation

telkins
Copy link
Contributor

@telkins telkins commented Jan 10, 2022

Completing the changes noted in #40285, I implemented the following (remaining) "check-like" methods on the Stringable class:

  • endsWith()
  • exactly()
  • is()
  • isAscii()
  • isUuid()
  • test()
  • startsWith()

Please let me know if there are any questions/concerns. 🤓

@huangdijia
Copy link
Contributor

huangdijia commented Jan 10, 2022

Can implement them by __call()? like:

    public function __call($name, $args)
    {
        $method = substr($name, 4);
        [$needles, $callback, $default] = $args;
        if ($this->$method($needles)) {
            $result = $callback($this);

            return $result ?? $this;
        } elseif ($default) {
            $result = $default($this);

            return $result ?? $this;
        }

        return $this;
    }

@telkins
Copy link
Contributor Author

telkins commented Jan 10, 2022

Can implement them by __call()? like:

    public function __call($name, $args)
    {
        $method = substr($name, 4);
        if ($this->$method(...$args)) {
            $result = $callback($this);

            return $result ?? $this;
        } elseif ($default) {
            $result = $default($this);

            return $result ?? $this;
        }

        return $this;
    }

Perhaps. I'd be a little reluctant to do it that way, though, because I don't know what problems might come about by using __call() for these methods.

It's a good suggestion, otherwise, though, as it's clear that all the methods have the exact same structure. I was a bit annoyed with that but hadn't taken the time to find a good way to DRY things up a bit while still keeping things straightforward, clear, and obvious.

I did consider (re)using when(), which might be a nice step toward cleaning a lot of the stuff up. I might have to play around with some of these ideas. 🤓

@telkins
Copy link
Contributor Author

telkins commented Jan 10, 2022

I quickly tossed out all of the repetitive elements and re-used when(). (I also applied this same change to the relatively new methods, whenContains() and whenContainsAll().)

I'm not comfortable using __call() to further minimize the repetition. I feel this is quite clean as-is. I'm happy to hear from others, however, who may feel differently. 🤓

@huangdijia
Copy link
Contributor

public function __call($name, $args)
{
    if (Str::startsWith($name, 'when')) {
        $method = lcfirst(substr($name, 4));
        if (in_array($method, ['isUuid', 'isAscii', ...])) {
            $refMethod = new \ReflectionMethod(self::class, $method);
            $argc = $refMethod->getNumberOfParameters();
            $params = array_slice($args, 0, $argc);
            $callack = $args[$argc];
            $default = $args[$argc + 1] ?? null;

            return $this->when($this->$method(...$params), $callback, $default);
        }
    }
}

@telkins

@taylorotwell taylorotwell merged commit de04718 into laravel:8.x Jan 11, 2022
@taylorotwell
Copy link
Member

Thanks for contributing to Laravel! ❤️

@telkins
Copy link
Contributor Author

telkins commented Jan 13, 2022

public function __call($name, $args)
{
    if (Str::startsWith($name, 'when')) {
        $method = lcfirst(substr($name, 4));
        if (in_array($method, ['isUuid', 'isAscii', ...])) {
            $refMethod = new \ReflectionMethod(self::class, $method);
            $argc = $refMethod->getNumberOfParameters();
            $params = array_slice($args, 0, $argc);
            $callack = $args[$argc];
            $default = $args[$argc + 1] ?? null;

            return $this->when($this->$method(...$params), $callback, $default);
        }
    }
}

@telkins

Hey, @huangdijia . Thanks for the updated suggestion. It looks pretty slick, but I'm still not sure I'm comfortable using __call() for this.

I'm sure you can create your own PR that changes the implementation for all these methods to this...and then see what Taylor and Crew say. 🤓

Take care... 🚀

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

Successfully merging this pull request may close these issues.

3 participants