Hacker News Reader

Comment by reikonomusha | Hacker News Reader

(Separate comment addressing the library itself.)

I find the naming of the classes off.

Why not call X as Any?

Why use nouns for some things, and verbs for others? I found Include and Exclude to be confusion. Inclusion and exclusion are relative to something. You include something with something else. I think Only and AnyBut would be better names.

Why call it Seq when it's closer to a Python Range? Seq usually means sequentiality.

Why not call Times as Repeat or Repetition?

It didn't look like this had any compatibility with existing regexes. I can't parse an existing regex into this library.

It looks like debugging could be a nightmare since it just passes things off to Python's re library. So if an error happens there, it will be tough to trace it to your original construction.

All of this class hierarchy building seems prime to replace with algebraic data types, which would probably cut the code down to just a fraction of the size.

View on HN

I also think the naming is really bad and can be improved. The central problem is that you're using action words for a descriptive API. You should use something like "Repeating" instead of "Times". Also, the parameters to Times are distinctly un-pythonic. I would probably make it provide this API:

   def Repeating(pattern, *args, min=None, max=None, exact=None):...

   Repeating(P) => P*

   Repeating(P, a, b) and
   Repeating(P, min=a, max=b) => P{a, b}

   Repeating(P, n) and
   Repeating(P, exact=n) => P{n, n}

   Repeating(P, min=n) => P{n,}

   Repeating(P, max=n) => P{0, n}

"Exclude" and "Include" suffer from the aforementioned problem of being procedural words for a declarative syntax. And they're easy to confuse with one another, having only two differing letters. And they don't make it clear that they match exactly one character. I would use "AnyBut" and "OneOf".

Why the split between Group and NamedGroup? Just add a name parameter to Group!

Seq as mentioned by parent is also problematic. Use Range or maybe Between or FromTo. To me FromTo flows the best, but Range is also ok.

I'm tempted to advocate adding operator overloading, so | would make a pattern that matches any one of two alternatives, * would be used for specifying repeats, etc. So you could write

   Digit = FromTo('0', '9')
   CapitalLetter = FromTo('A', 'Z')
   POCode = CapitalLetter * 2 + Digit * 3
   SmallLetter = FromTo('a', 'z')
   Letter = CapitalLetter or SmallLetter
   Name = CapitalLetter + Letter * (0, Inf)
   FirstName = Group(name='first', Name)
   LastName = Group(name='last', Name)
   FullName = FirstName + ' ' + LastName

However, implemented badly that can hurt readability much more than having to write functions everywhere.

> I'm tempted to advocate adding operator overloading, so | would make a pattern that matches any one of two alternatives, * would be used for specifying repeats, etc. So you could write

For repeat I think using indexing would make more sense though it might look somewhat unusual:

    Pattern[:]

would be *

    Pattern[1:]

would be + aka {1,}

    Pattern[5]

would be {5}

    Pattern[m:n]

would be {m,n}

It also uses a single operator for all repetition operations which is nice.

When i thought of Include and Exclude i had in mind it as a shorthand for IncludeFromTo and ExcludeFromTo.

About the Repeating/Repeat it could be just an idiomatic way of writing Include which stands for just one char thats fetched from a given sequence.

I may add synonyms for those classes, so, it is just a matter of adding:

Repeat = Times

etc.

you wouldnt use these structures in python code, you use them to build your regex then compile it and use the regex in your programs. It is possible to add some type safety and messages to help debugging which would be better than with natural regex syntax.

Seq stands for a-z or 0-9 which can be used only in Include and Exclude. When you write Include(Seq('a', 'm')) it means you are including a character from that sequence in a given position of your pattern. So if you have

Pattern(Include(Seq('a', 'm')), 'c')

That is sort of building the following set of strings.

ac bc cc . . .

When you use Exclude, it would be like you are picking a char that is not belonging to the sequence, like in:

Pattern(Exclude(Seq('0, 9')), c)

then you would end up with a set like:

ac bc cc . . .

It means, a set whose first character of the elements arent digits.

Seq is odd, to me Seq('a', 'm') says it matches 'am'. Why not overload X to support filtered forms instead? E.g X() is ".", X(from_, to_) is $from-$to, X(items) is [$items] and X(exclude) is [^$items]?

hmm, I think it is good, maybe p0 = X('a', 'z').

I like it.

Plus then you get to more add convenient overloads e.g. (X('a') | X('z')), (~X('q')), X[1:7] (I guess slices would be ranges, and indexes would be explicit numbers of repetitions), etc…

I think so i would ask you to open issues in crocs at github, so i can better keep track of suggestions and implement them.

Reply on news.ycombinator.com