CODEWITHSUNDEEP
ruby
KeepCalmAndCarryOn
KeepCalmAndCarryOn

Reputation: 9075

ruby do map and compact together

I'm new to ruby and this looks wrong but works fine

def get_internal_deps
  self.internal_dependencies = self.sources.map do |f| 
    s = File.open(File.join(self.dir, f)).grep(/\d{8}-\w{5}/)
    if s.length > 0
      {:file => f, :line => s}
    end
  end.compact
  #how crazy does that look?
end

So how do I do this without having an end.compact?

Upvotes: 2

Views: 1071

Answers (2)

Eric Duminil
Eric Duminil

Reputation: 54233

Some notes

  • Your method is called get_internal_deps, but it looks like it actually sets an instance variable.
  • You could define internal_dependencies and use caching.
  • In this case, you'd need to remove any attr_reader/writer/accessor for @internal_dependencies.
  • File.open(f) isn't really clean.
  • You don't need self in self.dir or self.sources
  • :line is an Array. Shouldn't it be called :lines?
  • 2 separate, short methods might be better than a bigger one.

Refactored code

def internal_dependencies
  @internal_dependencies ||= sources.map{|s| parse_dependency(s) }
                                    .reject{|h| h[:line].empty? }
end

private

def parse_dependency(source)
  {
    file: source,
    line: File.readlines(File.join(dir, source)).grep(/\d{8}-\w{5}/)
  }
end

Upvotes: 1

Aleksei Matiushkin
Aleksei Matiushkin

Reputation: 121000

To avoid compact one might use reduce (Enumerable#each_with_object in this particular case) instead of map:

def get_internal_deps
  self.internal_dependencies = sources.each_with_object do |f, acc|
    s = File.open(File.join(self.dir, f)).grep(/\d{8}-\w{5}/)
    acc << {:file => f, :line => s} if s.length > 0
  end
end

Also, note that an explicit self receiver might make sense in a case of assignment, but it is completely redundant in RHO (sources in this snippet.)

Upvotes: 1

Related Questions