8000 Suggestion: Replace your implementation of `Faker::String.from_regexp` with a dependency · Issue #328 · ffaker/ffaker · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Suggestion: Replace your implementation of Faker::String.from_regexp with a dependency #328

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.

Sign up for GitHub

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

Jump to bottom
Open
tom-lord opened this issue Jun 28, 2017 · 1 comment
Open

Suggestion: Replace your implementation of Faker::String.from_regexp with a dependency #328

tom-lord opened this issue Jun 28, 2017 · 1 comment

Comments

@tom-lord
Copy link
tom-lord commented Jun 28, 2017 •
edited
Loading

I recently stumbled across this gem, and noticed that you are using a custom implementation of Faker::String.from_regexp to generate strings from a regular expression:

ffaker/lib/ffaker/string.rb

Lines 19 to 28 in 49d2226

def from_regexp(exp)
result = ''
@last_token = nil
# Drop surrounding /'s and split into characters
tokens = exp.inspect[1...-1].split(//)
result << process_token(tokens) until tokens.empty?
result
end

This implementation is OK for the basics, but is far from comprehensive! There is an open issue about one such bug, and another open issue that's apparently unresolved.

The regexp language is vast and complicated; solving this problem for all possible regular expressions is far from easy. However, this library provides a much more complete solution. For backwards compatibility, you could implement this as:

module FFaker
  module String
    def self.from_regexp(exp)
      puts 'Warning: FFaker::String.from_regexp is deprecated. Use exp.random_example'
      exp.random_example
    end
  end
end

There are a couple of potential minor issues we could discuss before dropping in this replacement (e.g. Regexp#random_example could be a refinement instead of a global class extension?), but more the most part it should be a very simple change.
@lilsweetcaligula
Copy link
Contributor
lilsweetcaligula commented May 19, 2018 •
edited
Loading

Hello, just a random guy here looking to contribute.

In my opinion, improved support for FFaker::String.from_regexp could theoretically help easily resolve issues such as #353. In this case one would simply plug in a valid regex and wrap it in a method.

I am personally opposed to any sort of monkey patching in cases where it can otherwise be avoided, so if a dependency is to be introduced, I would recommend that the gem has its own namespace. Then FFaker::String.from_regexp could in theory be simply re-implemented as:

module FFaker::String
  def self.from_regexp(exp)
    RegexpToRandomString.generate(exp)
  end
end

Kind Regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@tom-lord @lilsweetcaligula
0