Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

Some simple suggestions for refactoring on your current code (without going into the implementation)

  1. Use Set over HashSet

Your original question says "Compute this in linear time using a HashSet.", but that usually doesn't mean you need to use HashSet as the return type. Set<String> result = new HashSet<>() works better in the sense that callers of your method will not need to know the underlying implementation. Also, you'll be free to change it to LinkedHashSet without changing the method signature.

  1. Variable names

Two points here.

i is usually used in for loops, so do consider using a better name in the method signature, e.g. permutateString(String str, int length).

Your count isn't actually counting anything, but to keep track of the last exclusive index for substring-ing (I think I just made a new word up!). For people who don't use String.substring() often, that line of code will instead read like you are creating sub-strings of increasing lengths.

  1. Looping

     for (int j = 0; j < str.length(); j++ ) {
     if (count > str.length()) { break; }
     ...
     }
    

    This can be better expressed as:

     for (int j = 0; j <= str.length() - length; j++ ) {
     result.add(str.substring(j, j + length));
     }
    

    Illustration:

     1234567890
     123
     234
     345
     456
     567
     678
     789
     890
    

    So, to extract 3-character sub-strings from a 10-character string, we need to loop from i = 0 to i = 7, or i <= string.length() - length for the latter.

  2. Unit testing

As mentioned elsewhere elsewhere, please consider using unit testing frameworks like JUnit or TestNG. :)

Some simple suggestions for refactoring on your current code (without going into the implementation)

  1. Use Set over HashSet

Your original question says "Compute this in linear time using a HashSet.", but that usually doesn't mean you need to use HashSet as the return type. Set<String> result = new HashSet<>() works better in the sense that callers of your method will not need to know the underlying implementation. Also, you'll be free to change it to LinkedHashSet without changing the method signature.

  1. Variable names

Two points here.

i is usually used in for loops, so do consider using a better name in the method signature, e.g. permutateString(String str, int length).

Your count isn't actually counting anything, but to keep track of the last exclusive index for substring-ing (I think I just made a new word up!). For people who don't use String.substring() often, that line of code will instead read like you are creating sub-strings of increasing lengths.

  1. Looping

     for (int j = 0; j < str.length(); j++ ) {
     if (count > str.length()) { break; }
     ...
     }
    

    This can be better expressed as:

     for (int j = 0; j <= str.length() - length; j++ ) {
     result.add(str.substring(j, j + length));
     }
    

    Illustration:

     1234567890
     123
     234
     345
     456
     567
     678
     789
     890
    

    So, to extract 3-character sub-strings from a 10-character string, we need to loop from i = 0 to i = 7, or i <= string.length() - length for the latter.

  2. Unit testing

As mentioned elsewhere, please consider using unit testing frameworks like JUnit or TestNG. :)

Some simple suggestions for refactoring on your current code (without going into the implementation)

  1. Use Set over HashSet

Your original question says "Compute this in linear time using a HashSet.", but that usually doesn't mean you need to use HashSet as the return type. Set<String> result = new HashSet<>() works better in the sense that callers of your method will not need to know the underlying implementation. Also, you'll be free to change it to LinkedHashSet without changing the method signature.

  1. Variable names

Two points here.

i is usually used in for loops, so do consider using a better name in the method signature, e.g. permutateString(String str, int length).

Your count isn't actually counting anything, but to keep track of the last exclusive index for substring-ing (I think I just made a new word up!). For people who don't use String.substring() often, that line of code will instead read like you are creating sub-strings of increasing lengths.

  1. Looping

     for (int j = 0; j < str.length(); j++ ) {
     if (count > str.length()) { break; }
     ...
     }
    

    This can be better expressed as:

     for (int j = 0; j <= str.length() - length; j++ ) {
     result.add(str.substring(j, j + length));
     }
    

    Illustration:

     1234567890
     123
     234
     345
     456
     567
     678
     789
     890
    

    So, to extract 3-character sub-strings from a 10-character string, we need to loop from i = 0 to i = 7, or i <= string.length() - length for the latter.

  2. Unit testing

As mentioned elsewhere, please consider using unit testing frameworks like JUnit or TestNG. :)

corrected formula for answer.
Source Link
h.j.k.
  • 19.3k
  • 3
  • 37
  • 93

Some simple suggestions for refactoring on your current code (without going into the implementation)

  1. Use Set over HashSet

Your original question says "Compute this in linear time using a HashSet.", but that usually doesn't mean you need to use HashSet as the return type. Set<String> result = new HashSet<>() works better in the sense that callers of your method will not need to know the underlying implementation. Also, you'll be free to change it to LinkedHashSet without changing the method signature.

  1. Variable names

Two points here.

i is usually used in for loops, so do consider using a better name in the method signature, e.g. permutateString(String str, int length).

Your count isn't actually counting anything, but to keep track of the last exclusive index for substring-ing (I think I just made a new word up!). For people who don't use String.substring() often, that line of code will instead read like you are creating sub-strings of increasing lengths.

  1. Looping

     for (int j = 0; j < str.length(); j++ ) {
     if (count > str.length()) { break; }
     ...
     }
    

    This can be better expressed as:

     for (int j = 0; j <= str.length() - length; j++ ) {
     result.add(str.substring(j, j + length));
     }
    

    Illustration:

     1234567890
     123
     234
     345
     456
     567
     678
     789
     890
    

    So, to extract 3-character sub-strings from a 10-character string, we need to loop from i = 0 to i = 7, or i <= string.length() - length for the latter.

  2. Unit testing

As mentioned elsewhere, please consider using unit testing frameworks like JUnit or TestNG. :)

Some simple suggestions for refactoring on your current code (without going into the implementation)

  1. Use Set over HashSet

Your original question says "Compute this in linear time using a HashSet.", but that usually doesn't mean you need to use HashSet as the return type. Set<String> result = new HashSet<>() works better in the sense that callers of your method will not need to know the underlying implementation. Also, you'll be free to change it to LinkedHashSet without changing the method signature.

  1. Variable names

Two points here.

i is usually used in for loops, so do consider using a better name in the method signature, e.g. permutateString(String str, int length).

Your count isn't actually counting anything, but to keep track of the last exclusive index for substring-ing (I think I just made a new word up!). For people who don't use String.substring() often, that line of code will instead read like you are creating sub-strings of increasing lengths.

  1. Looping

     for (int j = 0; j < str.length(); j++ ) {
     if (count > str.length()) { break; }
     ...
     }
    

    This can be better expressed as:

     for (int j = 0; j <= str.length() - length; j++ ) {
     result.add(str.substring(j, length));
     }
    

    Illustration:

     1234567890
     123
     234
     345
     456
     567
     678
     789
     890
    

    So, to extract 3-character sub-strings from a 10-character string, we need to loop from i = 0 to i = 7, or i <= string.length() - length for the latter.

  2. Unit testing

As mentioned elsewhere, please consider using unit testing frameworks like JUnit or TestNG. :)

Some simple suggestions for refactoring on your current code (without going into the implementation)

  1. Use Set over HashSet

Your original question says "Compute this in linear time using a HashSet.", but that usually doesn't mean you need to use HashSet as the return type. Set<String> result = new HashSet<>() works better in the sense that callers of your method will not need to know the underlying implementation. Also, you'll be free to change it to LinkedHashSet without changing the method signature.

  1. Variable names

Two points here.

i is usually used in for loops, so do consider using a better name in the method signature, e.g. permutateString(String str, int length).

Your count isn't actually counting anything, but to keep track of the last exclusive index for substring-ing (I think I just made a new word up!). For people who don't use String.substring() often, that line of code will instead read like you are creating sub-strings of increasing lengths.

  1. Looping

     for (int j = 0; j < str.length(); j++ ) {
     if (count > str.length()) { break; }
     ...
     }
    

    This can be better expressed as:

     for (int j = 0; j <= str.length() - length; j++ ) {
     result.add(str.substring(j, j + length));
     }
    

    Illustration:

     1234567890
     123
     234
     345
     456
     567
     678
     789
     890
    

    So, to extract 3-character sub-strings from a 10-character string, we need to loop from i = 0 to i = 7, or i <= string.length() - length for the latter.

  2. Unit testing

As mentioned elsewhere, please consider using unit testing frameworks like JUnit or TestNG. :)

edited for clarity.
Source Link
h.j.k.
  • 19.3k
  • 3
  • 37
  • 93

Some simple suggestions for refactoring on your current code (without going into the implementation)

  1. Use Set over HashSet

Your original question says "Compute this in linear time using a HashSet.", but that usually doesn't mean you need to use HashSet as the return type. Set<String> result = new HashSet<>() works better in the sense that callers of your method will not need to know the underlying implementation. Also, you'll be free to change it to LinkedHashSet without changing the method signature.

  1. Variable names

Two points here.

i is usually used in for loops, so do consider using a better name in the method signature, e.g. permutateString(String str, int length).

Your count isn't actually counting anything, but to keep track of the last exclusive index for substring-ing (I think I just made a new word up!). For people who don't use String.substring() often, that line of code will instead read like you are creating sub-strings of increasing lengths.

  1. Looping

     for (int j = 0; j < str.length(); j++ ) {
     if (count > str.length()) { break; }
     ...
     }
    

    This can be better expressed as:

     for (int j = 0; j <<= str.length() - length; j++ ) {
     result.add(str.substring(j, length));
     }
    

    Illustration:

     1234567890
     123
     234
     345
     456
     567
     678
     789
     890
    

    So, to extract 3-character sub-strings from a 10-character string, we need to loop from i = 0 to i = 7, or i <= string.length() - length for the latter.

  2. Unit testing

As mentioned elsewhere, please consider using unit testing frameworks like JUnit or TestNG. :)

Some simple suggestions for refactoring on your current code (without going into the implementation)

  1. Use Set over HashSet

Your original question says "Compute this in linear time using a HashSet.", but that usually doesn't mean you need to use HashSet as the return type. Set<String> result = new HashSet<>() works better in the sense that callers of your method will not need to know the underlying implementation. Also, you'll be free to change it to LinkedHashSet without changing the method signature.

  1. Variable names

Two points here.

i is usually used in for loops, so do consider using a better name in the method signature, e.g. permutateString(String str, int length).

Your count isn't actually counting anything, but to keep track of the last exclusive index for substring-ing (I think I just made a new word up!). For people who don't use String.substring() often, that line of code will instead read like you are creating sub-strings of increasing lengths.

  1. Looping

     for (int j = 0; j < str.length(); j++ ) {
     if (count > str.length()) { break; }
     ...
     }
    

    This can be better expressed as:

     for (int j = 0; j < str.length() - length; j++ ) {
     result.add(str.substring(j, length));
     }
    
  2. Unit testing

As mentioned elsewhere, please consider using unit testing frameworks like JUnit or TestNG. :)

Some simple suggestions for refactoring on your current code (without going into the implementation)

  1. Use Set over HashSet

Your original question says "Compute this in linear time using a HashSet.", but that usually doesn't mean you need to use HashSet as the return type. Set<String> result = new HashSet<>() works better in the sense that callers of your method will not need to know the underlying implementation. Also, you'll be free to change it to LinkedHashSet without changing the method signature.

  1. Variable names

Two points here.

i is usually used in for loops, so do consider using a better name in the method signature, e.g. permutateString(String str, int length).

Your count isn't actually counting anything, but to keep track of the last exclusive index for substring-ing (I think I just made a new word up!). For people who don't use String.substring() often, that line of code will instead read like you are creating sub-strings of increasing lengths.

  1. Looping

     for (int j = 0; j < str.length(); j++ ) {
     if (count > str.length()) { break; }
     ...
     }
    

    This can be better expressed as:

     for (int j = 0; j <= str.length() - length; j++ ) {
     result.add(str.substring(j, length));
     }
    

    Illustration:

     1234567890
     123
     234
     345
     456
     567
     678
     789
     890
    

    So, to extract 3-character sub-strings from a 10-character string, we need to loop from i = 0 to i = 7, or i <= string.length() - length for the latter.

  2. Unit testing

As mentioned elsewhere, please consider using unit testing frameworks like JUnit or TestNG. :)

Source Link
h.j.k.
  • 19.3k
  • 3
  • 37
  • 93
Loading
lang-java

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