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 1 down vote favorite

So I came across an interesting problem awhile back, and I finally got around to solving it. Basically, it needs to allow the user to provide two words, and have them progressively splice together, like so:

Word 1: 123
Word 2: abc

Result: 123abc
        12a3bc
        1a2b3c
        a1b2c3
        ab1c23
        abc123

The two words provided must maintain their order as they weave between eachother, and they should be arranged so that any two letters in one word are never separated by more than one letter from the other word.

I half expect to learn that I over-thought the problem, but I still think it's pretty slick. I'm open to all kinds of feedback.

EDIT: Here's the updated version. Old version is below:

!/usr/bin/ruby

class Array
  def swap!(a,b)
    self[a], self[b] = self[b], self[a]
    self
  end
end

class String
  def phase(other)
    if self.empty?
      [other]
    elsif other.empty?
      [self]
    else
      @word1 = self.split("")
      @word2 = other.split("")
      @combined_words = []

      @word2.each { |letter| @combined_words.push({:letter => nil, :word => nil}) }

      @word1.each do |letter|
        @combined_words.push({:letter => letter, :word => 1})
        @combined_words.push({:letter => nil,  :word => nil})
      end

      @word2.each { |letter| @combined_words.push({:letter => letter, :word => 2}) }
    end

    while @combined_words.include?({:letter => nil, :word => nil}) 
      nil_loc = @combined_words.rindex{ |addr| addr[:word].nil? }.to_i
      word2_subloc = @combined_words.drop(nil_loc).index{ |addr| addr[:word] == 2 }.to_i

      if word2_subloc == 0
        @combined_words.delete_at(nil_loc)
        print "\n\n"

        @combined_words.each do |addr|
        if not addr[:letter].nil?
          print addr[:letter]
        end
      end

      print "\n\n"

      else
        @combined_words.swap!(nil_loc, word2_subloc + nil_loc)
      end
    end
  end
end

puts "What is your first word?"

param1 = gets

puts "Cool, what is your second word?"

param2 = gets

puts param1.chomp.phase(param2.chomp)

Old version:

#!/usr/bin/ruby

class Array
    def swap!(a,b)
        self[a], self[b] = self[b], self[a]
        self
    end
end

class Phaser

    def initialize(word1, word2)

        raise unless word1.is_a?(String) && word2.is_a?(String)

        @word1 = word1.split("")
        @word2 = word2.split("")

        @combined_words = []

        @word2.each { |letter| @combined_words.push({:letter => nil, :word => nil}) }

        @word1.each { |letter|
            @combined_words.push({:letter => letter, :word => 1  })
            @combined_words.push({:letter => nil,    :word => nil})
        }

        @word2.each { |letter| @combined_words.push({:letter => letter, :word => 2}) }

    end

    def phase
        if !@combined_words.include?({:letter => nil, :word => nil})
            return
        else

            nil_loc = @combined_words.rindex{ |addr| addr[:word].nil? }.to_i

            word2_subloc = @combined_words.drop(nil_loc).index{ |addr| addr[:word] == 2 }.to_i

            if word2_subloc == 0

                @combined_words.delete_at(nil_loc)

                print "\n\n"

                @combined_words.each do |addr|
                    if not addr[:letter].nil?
                        print addr[:letter]
                    end
                end
            else
                @combined_words.swap!(nil_loc, word2_subloc + nil_loc)
            end

            phase

        end
    end
end

puts "What is your first word?"
param1 = gets

puts "Cool, what is your second word?"
param2 = gets

test_phase = Phaser.new(param1.chomp, param2.chomp)

test_phase.phase
share|improve this question

1 Answer 1

up vote 1 down vote

Some notes:

  • raise unless word1.is_a?(String) && word2.is_a?(String): Don't lose a second testing types of arguments, it's the caller's responsability to get it right.

  • Blank lines: You should be more careful with blank lines, used without consistency hinder readability severly. Here are my opinions on this.

  • I am tempted to write a bot that, upon finding an each, +=, delete, insert, value[x] = y or similar, automatically comments "this code looks terrible because it's in imperative style, try functional" :-) Sadly, most of the time the bot would be right. The problem is that you think about the problem in terms of how (do this, do that) instead of what, so variables are modified everywhere and it's just impossible to understand what the algorithm is doing. I've written at length about this subject, so if you're curious: FP in Ruby. Here the more natural approach seems a recursive functional algorithm.

That's how I'd write it:

class String
  def interleave(other)
    if self.empty?
      [other]
    elsif other.empty?
      [self]
    else
      interleaved1 = [self[0]].product(self[1..-1].interleave(other))
      interleaved2 = [other[0]].product(self.interleave(other[1..-1]))
      (interleaved1 + interleaved2).map(&:join)
    end
  end
end

p "123".interleave("abc")
#=> ["123abc", "12a3bc", "12ab3c", "12abc3", "1a23bc", "1a2b3c", "1a2bc3", "1ab23c", "1ab2c3", "1abc23", "a123bc", "a12b3c", "a12bc3", "a1b23c", "a1b2c3", "a1bc23", "ab123c", "ab12c3", "ab1c23", "abc123"]

In fact I'd implement the more generic Array#interleave (and go Array <-> String when needed). The changes in the implementation are minimal.

share|improve this answer
    
Thank you so much for the feedback, I'll definitely be reading those links when I get home. Just looking at your output though, your method doesn't seem to be producing the right results. For instance the 3rd element in your output is "12ab3c" but it should be "1a2b3c", which in your output doesn't occur until the 6th element –  bad_sample Jul 15 '13 at 16:37
    
@bad_sample: I noticed your output is shorter, I create all possible interleavings. Can you add a link to the specifications on the question? –  tokland Jul 15 '13 at 17:51
    
No link, it's just my own idea. I'll update my post to be more clear about the requireemnts –  bad_sample Jul 16 '13 at 18:01
    
I made some changes and uploaded the edited version based on some of your feedback. It's still imperative style but I did implement some of your changes –  bad_sample Jul 20 '13 at 20:18
    
@bad_sample: What I don't understand are the specifications of the problem: which outputs do you want and which don't. For example "1ab23c" seems a valid interleaving of 123 and abc, why isn't in your output? –  tokland Jul 20 '13 at 23:05

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.