1. Function name seems to be misleading. It returns true if no repeating character is found, so I find isNonRepeatedUsingArray
to be more appropriate.
2. Basic input checks - when I began trying your function, I set up a few tests (included in the final code) and some of them crashed. Your code should handle at least basic input values such as null, empty string, string with spaces.
public static boolean isNonRepeatedUsingArray(String inputStr) {
if(inputStr == null || inputStr.isEmpty())
return false;
String s = inputStr.replaceAll(" ", "").toUpperCase();
// other character stripping may be put here
3. Checking for duplicates - The algorithm itself can be improved by thinking in terms of set. I am not very familiar with Java language and I have used HashMap
data structure, but the bottom idea is the following:
if current element is already in the set, it is a duplicate. Else add it to the set.
This solution also has the advantage that avoids hardcoding the number of allowed characters (26).
Map<Character, Boolean> charDict = new HashMap<Character, Boolean>();
for (int i = 0; i < s.length(); i++) {
Character c = s.charAt(i);
if (charDict.containsKey(c))
return false;
charDict.put(c, true);
}
HashMap operations have a complexity of O(1)
, so function's overall complexity is O(n)
, where n is string length.
[edit]
Based on Bobby's
correct observation, a HashSet is a better choice (elements only, no key and values):
HashSet<Character> charSet = new HashSet<Character>();
for (int i = 0; i < s.length(); i++) {
Character c = s.charAt(i);
if (charSet.contains(c))
return false;
charSet.add(c);
}
4. Tests - it is a good habit to accompany a function with a test suite. It is particularly efficient when the code changes much (regressive testing) and helps to cover scenarios that are usually missed when developing (developers have a tendency to think in terms of making it work, not making it fail).
private static void printTestResult(String s) {
boolean testRes = isNonRepeatedUsingArray(s);
System.out.println("Repeating result for " + s + " is " + testRes);
}
public static void main(String []args){
printTestResult("a");
printTestResult("abcde");
printTestResult("some random text");
printTestResult("repeating");
printTestResult(null);
}
The final code
import java.util.*;
public class HelloWorld{
public static boolean isNonRepeatedUsingArray(String inputStr) {
if(inputStr == null || inputStr.isEmpty())
return false;
String s = inputStr.replaceAll(" ", "").toUpperCase();
Map<Character, Boolean> charDict = new HashMap<Character, Boolean>();
for (int i = 0; i < s.length(); i++) {
Character c = s.charAt(i);
if (charDict.containsKey(c))
return false;
charDict.put(c, true);
}
return true;
}
private static void printTestResult(String s) {
boolean testRes = isNonRepeatedUsingArray(s);
System.out.println("Repeating result for " + s + " is " + testRes);
}
public static void main(String []args){
printTestResult("a");
printTestResult("abcde");
printTestResult("some random text");
printTestResult("repeating");
printTestResult(null);
}
}
charArray
to be a data structure. \$\endgroup\$ – 200_success Feb 16 '16 at 19:54