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

One of the Class Methods not working as expected #65

Closed
Beleggrodion opened this issue Feb 12, 2020 · 8 comments
Closed

One of the Class Methods not working as expected #65

Beleggrodion opened this issue Feb 12, 2020 · 8 comments
Labels

Comments

@Beleggrodion
Copy link

Beleggrodion commented Feb 12, 2020

I don't know if it's a bug, or a Misunderstanding of the API. I used your Class in a project which i programmed for some tests last year in march.

I used the class like (this is one of the simpler usages)

if (!Pattern::of("/^000413/i")->matches($mac))

Now i see that this method is not existing anymore and was renamed to test, so i changed it to:

if (!Pattern::of("/^000413/i")->test($mac))

But now i always become "false" as return value, i only receive a true if i change the content of pattern 1:1 to the same value as the $mac value , so that it looks like a simple == value check.

One other usage is:

...

private $_macRegexList = array (
        "000413(25|28|2D|2F|34|36|37|3B|3D|3E|49|4A|4C|50|4E)[0-9A-F]{4}" => "snom300",
...
        "00041393[0-9A-F]{4}" => "snomD385",
);

...

foreach ($this->_macRegexList as $macRegex => $phoneModel)
{
    if (Pattern::of($macRegex)->test($mac))
    {
        return $phoneModel;
    }
}

Which now also not working anymore.

@danon
Copy link
Member

danon commented Feb 15, 2020

Hello @Beleggrodion, thanks for the Issue! :)

I used your Class in a project which i programmed for some tests last year in march.

I reckon you're not using versions 0.9.0 and 0.9.1, but using the one from master branch? :) Yes, the method matches() was renamed to test(), as you adequately noticed.

I don't know if it's a bug, or a Misunderstanding of the API

It's not a misunderstanding at all, because I didn't document it on t-regx.com yet.

But now i always become "false" as return value, i only receive a true if i change the content of pattern 1:1 to the same value as the $mac value , so that it looks like a simple == value check.

I know, that on t-regx.com webpage, it says that both undelimited and perl-compatible patterns are available (so pattern('^b', 'i') and pattern('/^b/i')).

But the I thought: "wait a second, what if someone uses pattern('/b/i', 'S')? Wouldn't that be confusing?". Or for example pattern('/a/') or pattern('#a#')? Is it a pattern with # delimiters? Or #a# pattern? :o

Clearly t-regx guessing, which is which is a bad idea :D

So I thought that it might be better, to introduce two methods: of() and pcre(). One would accept only the undelimited patterns, the other would only accept PCRE pattern. Like so:

Pattern::of('^B', 'i')->test('b');
Pattern::of('^B', 'i')->test('B');

Pattern::pcre('/^B/i')->test('b');
Pattern::pcre('/^B/i')->test('B');

Try using Pattern::pcre() for your delimited patterns and let me know if it works.

Also, please don't hesitate to share your thoughts on the topic :)

@danon danon added the question label Feb 15, 2020
@danon
Copy link
Member

danon commented Feb 15, 2020

Also

if (!Pattern::of("/^000413/i")->test($mac))

is the same as

if (Pattern::of("/^000413/i")->fails($mac))

;)

@Beleggrodion
Copy link
Author

I reckon you're not using versions 0.9.0 and 0.9.1, but using the one from master branch? :) Yes, the method matches() was renamed to test(), as you adequately noticed.

Yes. I use dev-master in my composer.json file :) Because its a personal project and no problem if some stuff not working for some time until its fixed on the one or other way.

It's not a misunderstanding at all, because I didn't document it on t-regx.com yet.

I assumed this, because i use the dev branch and no fixed version. ;-) So i decided to ask here.

I know, that on t-regx.com webpage, it says that both undelimited and perl-compatible patterns are available (so pattern('^b', 'i') and pattern('/^b/i')).
But the I thought: "wait a second, what if someone uses pattern('/b/i', 'S')? Wouldn't that be confusing?". Or for example pattern('/a/') or pattern('#a#')? Is it a pattern with # delimiters? Or #a# pattern? :o

If you see this in this way, is confusing because of so many possible options and variants.

Clearly t-regx guessing, which is which is a bad idea :D

Guessing in some functions is always not a good idea. In older php version this was a great problem i think. ;-)

So I thought that it might be better, to introduce two methods: of() and pcre(). One would accept only the undelimited patterns, the other would only accept PCRE pattern. Like so:

I think it's a good idea, so people wo want to use the one one method can use the one, the other can use the other method.

Try using Pattern::pcre() for your delimited patterns and let me know if it works.

i changed it in my code of the following test:

...
private $_macPattern = "^(000413[0-9A-F]{6})|(00087[bB][0-9A-F]{6})$";
...
if (!Pattern::pcre($this->_macPattern)->test($mac))
...

But i receive the following error message:

Argument 2 passed to TRegx\CleanRegex\Internal\Delimiter\Strategy\PcreIdentityStrategy::buildPattern() must be of the type string, null given, called in /www/vendor/rawr/t-regx/src/TRegx/CleanRegex/Internal/Delimiter/Delimiterer.php on line 26

I thin the pattern should be ok, because i copied it from a python script, which use the same pattern:

macPattern = re.compile("^(000413[0-9A-F]{6})|(00087[bB][0-9A-F]{6})$")

I know that fails exist, but i like the way with the ! more ;-) Perhaps i'm to old school for this.

@danon
Copy link
Member

danon commented Feb 17, 2020

For pattern "^(000413[0-9A-F]{6})|(00087[bB][0-9A-F]{6})$", you should go Pattern::of(), because it doesn't have delimiters.

Your earlier example, "/^000413/i" does have delimiters, so it should use Pattern::pcre().

Try:

private $_macPattern = "^(000413[0-9A-F]{6})|(00087[bB][0-9A-F]{6})$";
...
if (!Pattern::of($this->_macPattern)->test($mac))

and

if (!Pattern::pcre("/^000413/i")->test($mac))

Hope I helped :)

@danon
Copy link
Member

danon commented Feb 17, 2020

But you do bring an interesting point, a more proper exception should be raised for such usecase.

Thanks for the precise exception report :)

@danon
Copy link
Member

danon commented Feb 18, 2020

This is related to: #55

@Beleggrodion
Copy link
Author

Thanks. It works now :) After i changed some parts to of and some to pcre.

@danon
Copy link
Member

danon commented Feb 18, 2020

Nice, good to know!

I'm closing this issue now, but please feel free to ask/contact me with anything! :D

@danon danon closed this as completed Feb 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants