Checking whether two strings are isomorphic I was trying to solve a problem to determine if 2 strings are isomorphic. They are isomorphic when each character in string one can be replaced with another character in string 2. Two different characters may not map to the same value, and same character must not match to different values. Also the length of the strings must be same.

I've used exception for logic flow:

module Isomorphism

  class NotIsomorphicException &lt; StandardError
    attr_reader :object
    def initialize(object)
      @object = object
    end
  end

  def self.isomorphic? string_a,string_b
    begin
      self.isomorphism string_a,string_b
      true
    rescue NotIsomorphicException =&gt; _
      return false
    end
  end

  protected
  def self.isomorphism string_a,string_b
    raise NotIsomorphicException.new("Unequal length") unless string_a.size == string_b.size

    isomorphism_map = {}
    associations = string_a.chars.zip string_b.chars

    associations.each do |char_one,char_two|
      if(isomorphism_map.key? char_one)
        raise NotIsomorphicException.new("Single character needs to map to multiple characters, not isomorphic") unless isomorphism_map[char_one] == char_two
      elsif(isomorphism_map.value? char_two)
        raise NotIsomorphicException.new("Two characters map to same value")
      end
      isomorphism_map[char_one] = char_two
    end
  end

end


Please let me know if it's okay to code this way and any improvements that I might be able to make this better.

Following are the specs, just in case:

describe Isomorphism do
  it "should return true for single character words" do
    expect(Isomorphism.isomorphic? "t","d").to be true
  end

  it "should return true for empty strings" do
    expect(Isomorphism.isomorphic? "","").to be true
  end

  it "should return false if strings are of unequal length" do
    expect(Isomorphism.isomorphic? "dh","dhruv").to be false
  end

  it "should return false when 2 characters need to map to same character" do
    expect(Isomorphism.isomorphic? "ab","aa").to be false
  end

  it "should return true for egg and add" do
    expect(Isomorphism.isomorphic? "egg","add").to be true
  end

  it "should return false for foo and bar" do
    expect(Isomorphism.isomorphic? "foo","bar").to be false
  end

  it "should return true for paper and title" do
    expect(Isomorphism.isomorphic? "paper","title").to be true
  end

  it "should return false for aab and aaa" do
    expect(Isomorphism.isomorphic? "aab","aaa").to be false
  end
end
Looking at your isomorphism helper function…

The flow of control is kind of nasty.  In particular, burying an unless in here…

if …
  raise Exception.new(…) unless …
elsif …
  raise
end


… is too tricky, because:


You already have an if-elsif expression, and it would be better to work it into the same chain.
It's at the end of a very long line, where it is barely visible.
unless, being a negation, is automatically less preferable to an affirmative if.


Switching the naming convention from string_a, string_b to char_one, char_two is not ideal.  Why not char_a, char_b?  Or better yet, just a, b?

Doing Hash lookups in reverse using value? is not efficient.  You would be better off making a "forward" map and an "inverse" map.  See below for an even better way to decompose the problem.

As for the use of exceptions internally, I think it's a bad idea.  It's just a cumbersome way to return false with commentary.  You could just write a comment to avoid the overhead.  Better yet, if you write the code well, I don't think there is a need for any comment.  For example, in the implementation below, b != expected_b is pretty self-explanatory.

module Isomorphism

  def self.isomorphic?(string_a, string_b)
    surjective?(string_a, string_b) &amp;&amp; surjective?(string_b, string_a)
  end

  protected
  def self.surjective?(string_a, string_b)
    return false if string_a.size != string_b.size
    surjection = {}

    string_a.chars.zip(string_b.chars).each do |a, b|
      expected_b = surjection[a]
      if expected_b &amp;&amp; (b != expected_b)
        return false
      end
      surjection[a] = b
    end
    true
  end

  # Alternate implementation: shorter, but won't detect failure as quickly
  def self.surjective?(string_a, string_b)
    return false if string_a.size != string_b.size
    pairs = string_a.chars.zip(string_b.chars)
    surjection = Hash[pairs]
    pairs.all? { |a, b| surjection[a] == b }
  end

end