Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add Error test cases and simplify duration parsing logic #41

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

Merged
stuarthicks merged 3 commits into master from regexp
Oct 31, 2017
Merged

Conversation

@thomshutt
Copy link
Contributor

@thomshutt thomshutt commented Oct 31, 2017

No description provided.

mpd/duration.go Outdated
if str[offset] != 'P' {
return 0, errors.New("input duration does not have a valid prefix")
// Check that only the parts we expect exist and that everything's in the correct order
r := regexp.MustCompile(
Copy link
Contributor

@stuarthicks stuarthicks Oct 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move the regex initialisation outside the method? Compiling the regex on every invocation feels unnecessary.

Copy link
Contributor Author

@thomshutt thomshutt Oct 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, done!

mpd/duration.go Outdated
// Check that only the parts we expect exist and that everything's in the correct order
r := regexp.MustCompile(
"^P" + // Must start with a 'P'
"(\\d+D)?" + // We only allow Days for durations, not Months or Years
Copy link
Contributor

@stuarthicks stuarthicks Oct 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use named captures? I prefer not to read captures out by slice index.

Copy link
Contributor Author

@thomshutt thomshutt Oct 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go doesn't really support named captures without doing a double loop of an array of parameter names and one of parameters

Copy link

Coverage Status

Coverage increased (+0.6%) to 80.0% when pulling b7175ee on regexp into 732362d on master.

Copy link

Coverage Status

Coverage increased (+0.6%) to 80.0% when pulling 27f9fac on regexp into 732362d on master.

@stuarthicks stuarthicks merged commit 802a153 into master Oct 31, 2017
@stuarthicks stuarthicks deleted the regexp branch October 31, 2017 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stuarthicks stuarthicks stuarthicks approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /