Page 1 of 1

Looking for some opinion on some code I wrote [Updated]

PostPosted: Wed Jan 22, 2014 2:49 pm
by PurityLake
Hi all,

Recently I have been working on a port of a Python project called CommonRegex.

https://github.com/PurityLake/CommonRegex-Cobra

That is the link above. Please give me some honest opinions and features that should be added.

P.S.
I realise that I haven't added any tests yet but trust me they are the next thing that is being added to the program

Since Last Time
CommonRegex-Cobra has come quite a way and I'm expanding it more and more where I can but I need some people to test it out and give me feedback and ideas of what everyone wants.

Thanks for any and all comments you make on this project, it's still a work in progress so every little helps.

Re: Looking for some opinion on some code I wrote

PostPosted: Thu Jan 23, 2014 9:51 am
by nerdzero
Hey, that looks really useful. Cool stuff.

I notice a lot of the methods seem to be doing generally the same thing. Could you refactor so the public methods call a common protected one? Something like this maybe?

...
def _match(pattern as Regex, text as String) as List<of String>
results = List<of String>()
if text == "", return results
match as Match = pattern.match(text)
while match.success
for mat in match.groups
if '[mat]' <> ""
results.add('[mat]')
break
match = match.nextMatch to !
return results

def credit_cards(text as String) as List<of String>
"""
Returns a list of credit card numbers
"""
return _match(CommonRegex.credit_card, text)

def dates(text as String) as List<of String>
"""
Returns a list of dates
"""
return _match(CommonRegex.date, text)
...


I recommend you add tests first though. Then you will be sure it didn't break anything that used to work.

Re: Looking for some opinion on some code I wrote

PostPosted: Thu Jan 23, 2014 11:01 am
by PurityLake
nerdzero wrote:Hey, that looks really useful. Cool stuff.

I notice a lot of the methods seem to be doing generally the same thing. Could you refactor so the public methods call a common protected one? Something like this maybe?

...
def _match(pattern as Regex, text as String) as List<of String>
results = List<of String>()
if text == "", return results
match as Match = pattern.match(text)
while match.success
for mat in match.groups
if '[mat]' <> ""
results.add('[mat]')
break
match = match.nextMatch to !
return results

def credit_cards(text as String) as List<of String>
"""
Returns a list of credit card numbers
"""
return _match(CommonRegex.credit_card, text)

def dates(text as String) as List<of String>
"""
Returns a list of dates
"""
return _match(CommonRegex.date, text)
...


I recommend you add tests first though. Then you will be sure it didn't break anything that used to work.


Thanks for checking it out, I noticed that myself but to be honest it was testing each seperatly.

Also yes I know that I have forgot my testing. I am planning on having it done as soon as possible.

Thanks again for your feedback.

Re: Looking for some opinion on some code I wrote

PostPosted: Sat Jan 25, 2014 12:58 am
by hopscc
Re layout:
perhaps add a simple single unit/smoke test to each method
def match_date
test
# single unit test
....
body
#support code


++ features:
normal :) -dates ( dd-mm-yyyy)
fp numbers (nnnnn.nnnnn )
exponential n.nnnnn e n
10 powers n.nnnnn *10 *n ( 4.23 * 10 *2 = 423 - or whatever the std notation is)

"" or ''delimited string ( allowing or not embedded " and ', single line (multiline?)

names ? - firstname<space> secondname .... firstname <space> single-initial . <space> secondname

Re: Looking for some opinion on some code I wrote

PostPosted: Mon Jan 27, 2014 6:03 am
by PurityLake
hopscc wrote:Re layout:
perhaps add a simple single unit/smoke test to each method
def match_date
test
# single unit test
....
body
#support code


++ features:
normal :) -dates ( dd-mm-yyyy)
fp numbers (nnnnn.nnnnn )
exponential n.nnnnn e n
10 powers n.nnnnn *10 *n ( 4.23 * 10 *2 = 423 - or whatever the std notation is)

"" or ''delimited string ( allowing or not embedded " and ', single line (multiline?)

names ? - firstname<space> secondname .... firstname <space> single-initial . <space> secondname


As regards "normal" dates, that can be found.
If you want you can post an issue on the project page and I'll see what I can do