0

Doing some beginner problems for Java:

Given two strings, append them together (known as "concatenation") and return the result.

However, if the concatenation creates a double-char, then omit one of the chars, so "abc" and "cat" yields "abcat".

My code:

public static String conCat(String a, String b) {
    //abc (c) == cat (c)
    if (a.substring(a.length()-1,a.length()) == b.substring(0,1)) {
          
        //return (ab) + (cat)
        return a.substring(0,a.length()-2) + b;
         
    //cat (c) == abc (c)
    } else if(b.substring(0,1) == a.substring(a.length()-1,a.length())) {
              
        //return (abc) + (at)
        return a + b.substring(2,b.length());
              
    } else if (a.equals("") || b.equals("")) {
        return a + b;
    }
}

I don't understand why Eclipse can't recognise the String returns.

Community
  • 1
  • 1
Adz
  • 2,809
  • 10
  • 43
  • 61
  • 9
    What would you expect to be returned if all the `if` conditions are false, i.e. execution reaches the end of the method? – Jon Skeet Jun 02 '14 at 10:17
  • 10
    (Also, don't use `==` to compare strings - see http://stackoverflow.com/questions/513832) – Jon Skeet Jun 02 '14 at 10:17
  • Stylistic and controversial comment: I don't like stacked if/return/else. I find it clearer to not have the else. If we are returning anyway, there is no need for else. – Thilo Jun 02 '14 at 10:20
  • "What would you expect to be returned if all the if conditions are false, i.e. execution reaches the end of the method?" I am not a clever man. Thank you Jon. – Adz Jun 02 '14 at 10:22
  • Also second `if` seems pointless. – Ceiling Gecko Jun 02 '14 at 10:36

2 Answers2

6

First of all, you are comparing Strings with ==, which compares them by reference. This means that equal Strings might not return true. To avoid this problem, always use .equals() to compare Strings.

Second, keep in mind that your if statements are checked in the order specified. Since you want to check for empty strings first, you should put that one on top.

Third, you have to return something on all codepaths. If all of the if statements are false, you don't return anything. If you add else return a + b; you should get the desired behavior.

Furthermore, I suggest using a slightly different approach:

public static String conCat(String a, String b) {
    //If either String is empty, we want to return the other.
    if (a.isEmpty()) return b;
    else if (b.isEmpty()) return a;
    else {
        //If the last character of a is the same as the first character of b:
        //Since chars are primitives, you can (only) compare them with ==
        if (a.charAt(a.length()-1) == b.charAt(0))
            return a + b.subString(1);
        //Otherwise, just concatenate them.
        else
            return a + b;
    }
}

Note that you can omit the else blocks, since return will end the execution of the method there, so this will also work:

public static String conCat(String a, String b) {
    if (a.isEmpty()) return b;
    if (b.isEmpty()) return a;
    if (a.charAt(a.length()-1) == b.charAt(0)) return a + b.subString(1);
    return a + b;
}
Taeir
  • 108
  • 5
  • 1
    Great answer, much more readible than the original code. +1 if you delete all the `else` in there. – Thilo Jun 02 '14 at 10:32
  • With that `return a + b.substring(1);` you don't have to fiddle around with the length of a. Also it should be `a.charAt(a.length() - 1)` – Absurd-Mind Jun 02 '14 at 11:31
1

actually, it can. but all your return statements are depending on a condition, so there 'll be cases for which you haven't provided a return statement.

in those cases, the method won't return anything, even while it should return a String.

add:

return "";

or

return null;

to the end of your method and try again.

Stultuske
  • 9,296
  • 1
  • 25
  • 37