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

Multi part validation #5

Closed
wants to merge 9 commits into from
Closed

Conversation

gee-forr
Copy link

Hi there,

I came across an issue where I couldn't validate codes longer than 3 parts. Turns out, it wasnt documented, and I just needed to add the parts as a second positional argument.

This however is inconsistent with the interface for the generate method.

I've updated the validate method's arguments to match those of generates, and have also added a feature to set a default parts length.

I've also updated the readme (but only in english, I don't speak Korean unfortunately), and bumped the version.

I hope you find the changes to your liking :) Thanks again.

CouponCode.validate('YENH-UPJK-PTE0-20U6-QYME-RBK1', 6).wont_be_nil
CouponCode.validate('YENH-UPJK-PTE0-20U6-QYME-RBK2', 6).must_be_nil
CouponCode.validate('YENH-UPJK-PTE0-20U6-QYME-RBK1', parts: 6).wont_be_nil
CouponCode.validate('YENH-UPJK-PTE0-20U6-QYME-RBK2', parts: 6).must_be_nil

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

parts = code.scan(/[0-9A-Z]{#{LENGTH}}/)
def self.validate(orig, options = { parts: @@parts })
num_parts = options.delete(:parts)
code = orig.upcase.gsub(/[^0-9A-Z]+/, '')

Choose a reason for hiding this comment

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

Unnecessary spacing detected.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

def self.validate(orig, options = { parts: @@parts })
num_parts = options.delete(:parts)
code = orig.upcase.gsub(/[^0-9A-Z]+/, "")
parts = code.scan(/[0-9A-Z]{#{LENGTH}}/)

Choose a reason for hiding this comment

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

Unnecessary spacing detected.

@baxang
Copy link
Owner

baxang commented Sep 13, 2015

Thanks for contributing @gee-forr. Sorry for being late. I've been sick for the last couple days 😢

As you pointed out, I agree that those #generate and #validate's method signatures are not consistent and it should be addressed.

However I'd prefer to leave current interface as it is because changing #validate's argument doesn't seem to improve the library's usability significantly. Instead, I am working on adding configuration so that the users of this gem can change the number of parts, the length of each part, etc.

The end result will look like:

CouponCode.configure do |c|
  c.parts = 5
  c.length = 6
  c.separator = '-'
end

I believe this approach will provide with more convenient use cases.

You can find working in progress code at #6. Please feel free to create another PR for other implementation suggestions.

@baxang
Copy link
Owner

baxang commented Apr 8, 2016

I updated README to make that multi-part validation clearer for now: 0d40bb4. Thanks again for your time and effort and my apology for being late 😭 I will be adding configuration as soon as possible.

@baxang baxang closed this Apr 8, 2016
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