3
\$\begingroup\$

I have completed an implementation for finding the first unique character.

Is there any way I can improve it?

package com.datastructures;

public class FirstUniqueCharacter {

    public static void main(String args[]){

        String string = "neslihan";
        System.out.println(firstUniqueChar(string));
    }

    private static int firstUniqueChar(String string) {

        int freq[] = new int[26];

        for(int i=0; i< string.length() ;i++){
            freq[string.charAt(i) - 'a'] ++;
        }
        for (int j =0;j<string.length();j++){
            ;
            System.out.println(string.charAt(j)-0);
            System.out.println('a'-0);
            if(freq[string.charAt(j) - 'a'] == 1) {
                return j;
            }
        }
        return -1;
    }
}
\$\endgroup\$
  • \$\begingroup\$ Your code does not return the first unique character. It returns the index of the first unique character. \$\endgroup\$ – RobAu Jul 29 at 14:56
  • \$\begingroup\$ You didn't specify what would you like to improve, performance or readability? Usually, you need to sacrifice one to get another. \$\endgroup\$ – Cezary Butler Jul 29 at 19:53
5
\$\begingroup\$

As from @RobAu 's comment the title is misleading because you returns the index of the first unique character in your frequencies array so that the index 0 refers to the 'a' char, 1 refers to the 'b' char, etc., making the assumption that your string just contains only letter chars coerent with the ascii table.

In your code you make use of string methods length() and charAt to iterate over the string , you can iterate directly over one char array like below:

private static int firstUniqueChar(String string) {
    int freq[] = new int[26];
    char[] arr = string.toCharArray();

    for(char c : arr) { ++freq[c - 'a']; }

    //other instructions
}

You are using subtraction by 0 in these lines:

System.out.println(string.charAt(j)-0);
System.out.println('a'-0);

I presume you are using this subtraction because you want to print the int value of the char, you can solve this issue casting the char value to int. Below my version of your code:

private static int firstUniqueChar(String string) {
    int freq[] = new int[26];
    char[] arr = string.toCharArray();

    for(char c : arr){
        ++freq[c - 'a'];
    }
        
    for (int j =0;j < arr.length; ++j){

        System.out.println((int) arr[j]);
        System.out.println((int) 'a');

        if(freq[arr[j] - 'a'] == 1) {
            return j;
        }
    }
    
    return -1;
}

From your code I have the impression at the moment you are mixing different concepts in an unclear manner.

| improve this answer | |
\$\endgroup\$
4
\$\begingroup\$

Clean up your code

Don't leave in superfluous code (such as the extra ; and subtraction by 0) and debug output in your code.

Also format the code consistently, preferably using conventions outlined in a Style Guide, such as: Put spaces around operators, put a space between keywords (such as if) and the following (, etc.

Document your code

Especially because your task doesn't say, clearly document what your code takes/allows as input and what it returns. Currently your code aborts with an unhelpful exception if it receives an unexpected input such as any non-lowercase letter. Unless this needs to be super optimized (it doesn't) it would be better to check for and reject invalid input.

Use high level Java features

Unless you are specifically learning to use low-level feature (such as arrays) or are writing super optimized code (you aren't) consider using the full range of the Java API. For instance in this case it would make sense to use a Map<Char, Integer> to hold the character count instead of an array. This has for one the advantage that it can hold all characters and not only lower case ones.

Alternative algorithm

There is a slightly better different alternative that only loops once over the string. It requires, aside from the array/Map with the character count, additionally a queue.

In the loop for the current char:

  • Increase the character count
  • Put the index in the end of the queue
  • Remove all indices from start of the queue, which point to a character that has more than one count

After the loop, the index is the first position of the queue is the solution.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ You should explain slightly better; because you intend in terms of cpu-speed? Memory wise it is worse imho. Also, real-world performance wise it is too be seen in which cases it is faster; because of auto-boxing and referencing iterating the String twice might just be the faster solution... \$\endgroup\$ – RobAu Jul 29 at 15:33
  • \$\begingroup\$ @RobAu You are right, it's not better. I've changed the wording. \$\endgroup\$ – RoToRa Jul 30 at 14:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.