How to let my code also check strings with length 0?

Question

I am trying to remove the letter x if it is present as the first or last character of any given string. Why does my code not check strings with length 0 even though I have accounted for it in my code?

I have already tried using an if statement to check if the length is 0 and, if so, to return the original string. I have also tried returning an empty string with the same value.

public String withoutX(String str) {
  if (((str.charAt(0)=='x') || str.charAt(str.length()-1) == 'x') && str.length()>=2){
    if (str.charAt(0)=='x'){
        str = str.substring(1,str.length());
    } if (str.charAt(str.length()-1) == 'x'){
        str = str.substring(0,str.length()-1);
    }
  } if (str.length()==1 && str == "x"){
      return "";
  } if (str.length()==0){
      return str;
    // the above if statement (length = 0) does not work
  } else{
      return str;
  }
}

The expected result is for it to return the string without the letter x. This has been achieved for all strings, except an empty one, where it says that the index is out of range.

First if causes an exception when you try to access index 0 of an empty string — Roy Shahaf, Dec 22 '18 at 21:37
`str == "x"` will almost always fail; see https://stackoverflow.com/questions/513832/how-do-i-compare-strings-in-java. — VGR, Dec 22 '18 at 21:42
You might find `String.startsWtih` and `.endsWith` handy. (And avoiding the strange formatting of `if` statements.) Also, you can do this with a regex if you so wish. And `&&` is lazy but the left hand is always evaluated - you probably wanted your length check on the other side. — Tom Hawtin - tackline, Dec 22 '18 at 21:56
Hava a look at this answer. You check a lot of the edge cases twice: https://stackoverflow.com/a/53899546/3103271 — Charlie, Dec 22 '18 at 22:05
I wonder that a `Collection`might not ease some pain of dealing with an `Array` or `String` here. — Thufir, Dec 23 '18 at 02:45

Andrew Tobilko · Answer 1 · 2018-12-22T21:59:32.087

1

if (str.length() == 0) {
    return str;
}

It should be at the top of the method body. Or, at least, it should go prior to getting str.charAt(0).

There's String#isEmpty.
The way you compared Strings is wrong. Use equals instead of ==.
Always validate the length before accessing a letter by index. Make sure that index in a string exists.

edited Dec 22 '18 at 21:59

answered Dec 22 '18 at 21:37

Andrew Tobilko

48,120
14
91
142

forpas · Answer 2 · 2018-12-22T22:17:00.683

In this line:

if (((str.charAt(0)=='x') || str.charAt(str.length()-1) == 'x') && str.length()>=2)

you do check for the length of the string but after you check the indexes.
Change to:

if (str.length()>=2 && (str.charAt(0)=='x' || str.charAt(str.length()-1) == 'x'))

if str.length()>=2 is false the other conditions will not be checked because of Short Circuit Evaluation.
Also use equals() to compare strings and not ==.
I vwould write your method like this:

public String withoutX(String str) {
    if (str.length() == 0 || (str.length() == 1 && str.equals("x")))
        return "";

    if (str.charAt(0) == 'x') {
        str = str.substring(1, str.length());
    }
    if (str.charAt(str.length() - 1) == 'x') {
        str = str.substring(0, str.length() - 1);
    }
    return str;
}

I kept your logic and removed unnecessary code.

Charlie · Answer 3 · 2018-12-23T12:32:50.210

public String withoutX(String str) {
  if (!str.isEmpty() && str.charAt(0)=='x'){
    str = str.substring(1, str.length());
  } 

  if (!str.isEmpty() && str.charAt(str.length()-1) == 'x'){
    str = str.substring(0,str.length()-1);
  }

  return str;
}

You can first check, if the str is empty and remove the first x. Then check the size for greater than 1 and remove the last x. This leads to less and more readable code.

score 0 · Answer 4 · answered Dec 22 '18 at 22:52

I think this is a better implementation of what you are trying to do

public String withoutX(String str) {

      if (str.startsWith("x")){ 
        str = str.replaceFirst("x", "");
      } 

      if (str.endsWith("x")){
        str = str.substring(0,str.lastIndexOf("x"));
      }

      return str;
}

score 0 · Answer 5 · answered Dec 23 '18 at 00:45

0

A simpler approach without making many corner-cases checking is to use regex to remove the leading and trail x - if it exists. Here's the code:

public String withoutX(String str){
    return "".equals(str)? str: str.replaceAll("(^x|x$)", "");
}

answered Dec 23 '18 at 00:45

Eslam Nawara

645
3
8

score 0 · Answer 6 · answered Dec 23 '18 at 04:59

Your code is checking for a x before you check if the string is empty. You should move

if (str.length()==0){
      return str;
    // the above if statement (length = 0) does not work
  }

to the first line of the method. Besides that your code is great.

How to let my code also check strings with length 0?

6 Answers6