current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite
1

This is method which creates an array of all possible consecutive substrings from from a string

def get_seperated_tokens(query)
  result = []
  length = query.split.count
  tokens = query.downcase.strip.split(' ')
  length.times do |i|
    length.times do |j|
      result << tokens[i..j].join(' ') if j >= i
    end
  end
  result
end

To get a better idea I have added rspec for it

describe "#get_seperated_tokens" do
  it "returns an array of seperated tokens" do
    query = 'ruby is awesome'
    result = ['ruby','is', 'awesome', 'ruby is', 'is awesome','ruby is awesome']
    expect(get_seperated_tokens(query)).to include(*result)
  end
  it "returns an array of seperated tokens" do
    query = 'red blue iphones'
    result = ['red','blue', 'iphones', 'red blue', 'blue iphones','red blue iphones']
    expect(get_seperated_tokens(query)).to include(*result)
  end
end
share|improve this question
    
And? What is your question? –  Michael Szyndel Jul 19 '13 at 9:56
    
A better ruby way to do it –  Yatish Mehta Jul 19 '13 at 11:01
    
ruby-doc.org/core-1.9.3/Array.html#method-i-combination –  Michael Szyndel Jul 19 '13 at 11:08

2 Answers 2

up vote 1 down vote accepted 
def get_separated_tokens query
  tokens = query.split
  (0..tokens.size).to_a.combination(2).map{ |i,j| tokens[i...j].join " " }
end

Oh, @Michael Szyndel mentioned this method in comments.

share|improve this answer
    
In fact Michael wrote an answer with combination (now deleted), the problem is that OP needs contiguous combinations, so this won't do. –  tokland Jul 21 '13 at 9:17
    
@tokland, I don't get you. Did you run this code? It produces the same as OP's code. –  Nakilon Jul 22 '13 at 10:29
    
oh, you're right, you use combinations, but with a slice later, which is right. +1 –  tokland Jul 22 '13 at 10:34
up vote 1 down vote

Are you familiar with functional programming? (my page on the subject: FP with Ruby). Your code feels clunky because, well, imperative programming is clunky (recommended reading: Can Programming Be Liberated From The Von Neumann Style?).

You just need to re-write the code without times (as it's being used as an each), mutable variables (result = []), inplace operations (<<) and inline conditionals with side-effects (do_something if j >= i). I'd write:

def get_separated_tokens(query)
  tokens = query.split
  (0...tokens.size).flat_map do |istart|
    (istart...tokens.size).map do |iend|
      tokens[istart..iend].join(" ")
    end
  end
end

p get_separated_tokens("ruby is awesome")
#["ruby", "ruby is", "ruby is awesome", "is", "is awesome", "awesome"]
share|improve this answer
    
Hey thanks, Help a lot –  Yatish Mehta Jul 21 '13 at 4:25
    
You can select Nakilon's answer, I calculated the combinations to build ranges, we have Array#combination. –  tokland Jul 22 '13 at 10:51

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.