StackOverflow2013

Note that there are some explanatory texts on larger screens.

plurals

PO
text
Body
copied!<p>Your code is, I'm sorry to say, very C-like.</p> <p>A Java <code>String</code> is not a <code>char[]</code>. You say you want to remove duplicates from a <code>String</code>, but you take a <code>char[]</code> instead.</p> <p>Is this <code>char[]</code> <code>\0</code>-terminated? Doesn't look like it because you take the whole <code>.length</code> of the array. But then your algorithm tries to <code>\0</code>-terminate a portion of the array. What happens if the arrays contains no duplicates?</p> <p>Well, as it is written, your code actually throws an <code>ArrayIndexOutOfBoundsException</code> on the last line! There is no room for the <code>\0</code> because all slots are used up!</p> <p>You can add a check not to add <code>\0</code> in this exceptional case, but then how are you planning to use this code anyway? Are you planning to have a <code>strlen</code>-like function to find the first <code>\0</code> in the array? And what happens if there isn't any? (due to all-unique exceptional case above?).</p> <p>What happens if the original <code>String</code>/<code>char[]</code> contains a <code>\0</code>? (which is perfectly legal in Java, by the way, see <a href="http://java.sun.com/docs/books/jls/third_edition/html/arrays.html#10.9" rel="noreferrer">JLS 10.9 An Array of Characters is Not a String</a>)</p> <p>The result will be a mess, and all because you want to do everything C-like, and in place without any additional buffer. Are you sure you really need to do this? Why not work with <code>String</code>, <code>indexOf</code>, <code>lastIndexOf</code>, <code>replace</code>, and all the higher-level API of <code>String</code>? Is it provably too slow, or do you only suspect that it is?</p> <p>"Premature optimization is the root of all evils". I'm sorry but if you can't even understand what the original code does, then figuring out how it will fit in the bigger (and messier) system will be a nightmare.</p> <hr> <p>My minimal suggestion is to do the following:</p> <ul> <li>Make the function takes and returns a <code>String</code>, i.e. <code>public static String removeDuplicates(String in)</code></li> <li>Internally, works with <code>char[] str = in.toCharArray();</code></li> <li>Replace the last line by <code>return new String(str, 0, tail);</code></li> </ul> <p>This does use additional buffers, but at least the interface to the rest of the system is much cleaner.</p> <hr> <p>Alternatively, you can use <code>StringBuilder</code> as such:</p> <pre><code>static String removeDuplicates(String s) { StringBuilder noDupes = new StringBuilder(); for (int i = 0; i < s.length(); i++) { String si = s.substring(i, i + 1); if (noDupes.indexOf(si) == -1) { noDupes.append(si); } } return noDupes.toString(); } </code></pre> <p>Note that this is essentially the same algorithm as what you had, but much cleaner and without as many little corner cases, etc.</p>

Querying!

Guidance

An individual column

Larger individual text columns get their own page to allow for proper reading.

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload