Skip to main content
Code Review

Return to Answer

simplification
Source Link
200_success
  • 145.5k
  • 22
  • 190
  • 479

Your build two HashMap<Character, Integer>, then two ArrayList<Integer>, then sort the lists, then compare the lists. Your logic is too complicated, I think. Furthermore, you aren't taking advantage of the symmetry in the problem — nearly every line of code appears in duplicate. Even worse, the code doesn't even do what you claim; rather, it tests whether the inputs are isomorphic anagrams of each other.

Going by the definition, we can just build a character-to-character map from one string to the other, and fail as soon as we encounter an unexpected character. This should be O(n), and possibly faster if a mismatch is detected early.

The code works equally well on any pair of CharSequences, not just Strings, so we might as well generalize.

import java.util.HashMap;
import java.util.Map;
public class IsomorphicStrings {
 public static boolean areIsomorphic(CharSequence s1, CharSequence s2) {
 return isSurjective(s1, s2) && isSurjective(s2, s1);
 }
 private static boolean isSurjective(CharSequence s1, CharSequence s2) {
 if (s1.length() != s2.length()) return false;
 Map<Character, Character> surjection = new HashMap<>();
 for (int i = 0; i < s1.length(); i++) {
 char c1 = s1.charAt(i);
 char c2 = s2.charAt(i);
 Character expected2prev = surjection.getput(c1, c2);
 if (expected2prev != null && c2prev != expected2c2) {
 return false;
 }
 surjection.put(c1, c2);
 }
 return true;
 }
 public static void main(String[] args) {
 System.out.println(areIsomorphic(args[0], args[1]));
 }
}

Your build two HashMap<Character, Integer>, then two ArrayList<Integer>, then sort the lists, then compare the lists. Your logic is too complicated, I think. Furthermore, you aren't taking advantage of the symmetry in the problem — nearly every line of code appears in duplicate. Even worse, the code doesn't even do what you claim; rather, it tests whether the inputs are isomorphic anagrams of each other.

Going by the definition, we can just build a character-to-character map from one string to the other, and fail as soon as we encounter an unexpected character. This should be O(n), and possibly faster if a mismatch is detected early.

The code works equally well on any pair of CharSequences, not just Strings, so we might as well generalize.

import java.util.HashMap;
import java.util.Map;
public class IsomorphicStrings {
 public static boolean areIsomorphic(CharSequence s1, CharSequence s2) {
 return isSurjective(s1, s2) && isSurjective(s2, s1);
 }
 private static boolean isSurjective(CharSequence s1, CharSequence s2) {
 if (s1.length() != s2.length()) return false;
 Map<Character, Character> surjection = new HashMap<>();
 for (int i = 0; i < s1.length(); i++) {
 char c1 = s1.charAt(i);
 char c2 = s2.charAt(i);
 Character expected2 = surjection.get(c1);
 if (expected2 != null && c2 != expected2) {
 return false;
 }
 surjection.put(c1, c2);
 }
 return true;
 }
 public static void main(String[] args) {
 System.out.println(areIsomorphic(args[0], args[1]));
 }
}

Your build two HashMap<Character, Integer>, then two ArrayList<Integer>, then sort the lists, then compare the lists. Your logic is too complicated, I think. Furthermore, you aren't taking advantage of the symmetry in the problem — nearly every line of code appears in duplicate. Even worse, the code doesn't even do what you claim; rather, it tests whether the inputs are isomorphic anagrams of each other.

Going by the definition, we can just build a character-to-character map from one string to the other, and fail as soon as we encounter an unexpected character. This should be O(n), and possibly faster if a mismatch is detected early.

The code works equally well on any pair of CharSequences, not just Strings, so we might as well generalize.

import java.util.HashMap;
import java.util.Map;
public class IsomorphicStrings {
 public static boolean areIsomorphic(CharSequence s1, CharSequence s2) {
 return isSurjective(s1, s2) && isSurjective(s2, s1);
 }
 private static boolean isSurjective(CharSequence s1, CharSequence s2) {
 if (s1.length() != s2.length()) return false;
 Map<Character, Character> surjection = new HashMap<>();
 for (int i = 0; i < s1.length(); i++) {
 char c1 = s1.charAt(i);
 char c2 = s2.charAt(i);
 Character prev = surjection.put(c1, c2);
 if (prev != null && prev != c2) {
 return false;
 }
 }
 return true;
 }
 public static void main(String[] args) {
 System.out.println(areIsomorphic(args[0], args[1]));
 }
}
added 128 characters in body
Source Link
200_success
  • 145.5k
  • 22
  • 190
  • 479

Your build two HashMap<Character, Integer>, then two ArrayList<Integer>, then sort the lists, then compare the lists. Your logic is too complicated, I think. Furthermore, you aren't taking advantage of the symmetry in the problem — nearly every line of code appears in duplicate. Even worse, the code doesn't even do what you claim; rather, it tests whether the inputs are isomorphic anagrams of each other.

Going by the definition, we can just build a character-to-character map from one string to the other, and fail as soon as we encounter an unexpected character. This should be O(n), and possibly faster if a mismatch is detected early.

The code works equally well on any pair of CharSequences, not just Strings, so we might as well generalize.

import java.util.HashMap;
import java.util.Map;
public class IsomorphicStrings {
 public static boolean areIsomorphic(CharSequence s1, CharSequence s2) {
 return isSurjective(s1, s2) && isSurjective(s2, s1);
 }
 private static boolean isSurjective(CharSequence s1, CharSequence s2) {
 if (s1.length() != s2.length()) return false;
 Map<Character, Character> surjection = new HashMap<>();
 for (int i = 0; i < s1.length(); i++) {
 char c1 = s1.charAt(i);
 char c2 = s2.charAt(i);
 Character expected2 = surjection.get(c1);
 if (expected2 != null && c2 != expected2) {
 return false;
 }
 surjection.put(c1, c2);
 }
 return true;
 }
 public static void main(String[] args) {
 System.out.println(areIsomorphic(args[0], args[1]));
 }
}

Your build two HashMap<Character, Integer>, then two ArrayList<Integer>, then sort the lists, then compare the lists. Your logic is too complicated, I think. Furthermore, you aren't taking advantage of the symmetry in the problem — nearly every line of code appears in duplicate.

Going by the definition, we can just build a character-to-character map from one string to the other, and fail as soon as we encounter an unexpected character. This should be O(n), and possibly faster if a mismatch is detected early.

The code works equally well on any pair of CharSequences, not just Strings, so we might as well generalize.

import java.util.HashMap;
import java.util.Map;
public class IsomorphicStrings {
 public static boolean areIsomorphic(CharSequence s1, CharSequence s2) {
 return isSurjective(s1, s2) && isSurjective(s2, s1);
 }
 private static boolean isSurjective(CharSequence s1, CharSequence s2) {
 if (s1.length() != s2.length()) return false;
 Map<Character, Character> surjection = new HashMap<>();
 for (int i = 0; i < s1.length(); i++) {
 char c1 = s1.charAt(i);
 char c2 = s2.charAt(i);
 Character expected2 = surjection.get(c1);
 if (expected2 != null && c2 != expected2) {
 return false;
 }
 surjection.put(c1, c2);
 }
 return true;
 }
 public static void main(String[] args) {
 System.out.println(areIsomorphic(args[0], args[1]));
 }
}

Your build two HashMap<Character, Integer>, then two ArrayList<Integer>, then sort the lists, then compare the lists. Your logic is too complicated, I think. Furthermore, you aren't taking advantage of the symmetry in the problem — nearly every line of code appears in duplicate. Even worse, the code doesn't even do what you claim; rather, it tests whether the inputs are isomorphic anagrams of each other.

Going by the definition, we can just build a character-to-character map from one string to the other, and fail as soon as we encounter an unexpected character. This should be O(n), and possibly faster if a mismatch is detected early.

The code works equally well on any pair of CharSequences, not just Strings, so we might as well generalize.

import java.util.HashMap;
import java.util.Map;
public class IsomorphicStrings {
 public static boolean areIsomorphic(CharSequence s1, CharSequence s2) {
 return isSurjective(s1, s2) && isSurjective(s2, s1);
 }
 private static boolean isSurjective(CharSequence s1, CharSequence s2) {
 if (s1.length() != s2.length()) return false;
 Map<Character, Character> surjection = new HashMap<>();
 for (int i = 0; i < s1.length(); i++) {
 char c1 = s1.charAt(i);
 char c2 = s2.charAt(i);
 Character expected2 = surjection.get(c1);
 if (expected2 != null && c2 != expected2) {
 return false;
 }
 surjection.put(c1, c2);
 }
 return true;
 }
 public static void main(String[] args) {
 System.out.println(areIsomorphic(args[0], args[1]));
 }
}
Source Link
200_success
  • 145.5k
  • 22
  • 190
  • 479

Your build two HashMap<Character, Integer>, then two ArrayList<Integer>, then sort the lists, then compare the lists. Your logic is too complicated, I think. Furthermore, you aren't taking advantage of the symmetry in the problem — nearly every line of code appears in duplicate.

Going by the definition, we can just build a character-to-character map from one string to the other, and fail as soon as we encounter an unexpected character. This should be O(n), and possibly faster if a mismatch is detected early.

The code works equally well on any pair of CharSequences, not just Strings, so we might as well generalize.

import java.util.HashMap;
import java.util.Map;
public class IsomorphicStrings {
 public static boolean areIsomorphic(CharSequence s1, CharSequence s2) {
 return isSurjective(s1, s2) && isSurjective(s2, s1);
 }
 private static boolean isSurjective(CharSequence s1, CharSequence s2) {
 if (s1.length() != s2.length()) return false;
 Map<Character, Character> surjection = new HashMap<>();
 for (int i = 0; i < s1.length(); i++) {
 char c1 = s1.charAt(i);
 char c2 = s2.charAt(i);
 Character expected2 = surjection.get(c1);
 if (expected2 != null && c2 != expected2) {
 return false;
 }
 surjection.put(c1, c2);
 }
 return true;
 }
 public static void main(String[] args) {
 System.out.println(areIsomorphic(args[0], args[1]));
 }
}
lang-java

AltStyle によって変換されたページ (->オリジナル) /