0

I am completely new to programming. Can you give me some tips on how to improve my code?
The problem was:
Given an array of strings, return a new array without the strings that are equal to the target string. One approach is to count the occurrences of the target string, make a new array of the correct length, and then copy over the correct strings.
And my code:

public String[] wordsWithout(String[] words, String target) {
  int numberOfTargets = 0;

  for (int i = 0; i < words.length; i++){
    if ( words[i].equals(target) ) numberOfTargets++;
  }

  String[] result = new String[words.length - numberOfTargets];

  for (int i = 0; i < words.length - numberOfTargets; i++){ // 1
    result[i] = "0";                                        // 1 
  }                                                         // 1 

  for (int i = 0; i < words.length; i++){
    if ( !words[i].equals(target) ){
      int j = 0;                         // 2   
      while ( !result[j].equals("0") ){  // 2
        j++;                             // 2 
      }                                  // 2
      result[j] = words[i];
    } 
  }
  return result;
}

Example of how code works:

wordsWithout(["aa", "ab", "ac", "aa"], "aa") → ["ab", "ac"]

I know that new array of ints is filled by zeros dy default. What about new array of Strings? I had to artificially fill it by zeros in part marked as //1, so that I could "scroll" to the right element, when I have to add elements to my new array in part marked as //2.
My code seems to be kind of awkward. Are there any standard methods or general ways to improve my code?

Alex
  • 23
  • 4
  • Well, you can use `Arrays.fill(result, "0");` to avoid the for loop. Arrays of objects are always initialized to null, just like primitive arrays are initialized to 0 or in boolean's case `false`. However this question belongs to Code Review, not StackOverflow. – Kayaman Apr 21 '16 at 12:25
  • You'll find the answer here : http://stackoverflow.com/questions/7940337/remove-a-specific-string-from-an-array-of-string As a side-note, if you're a beginner, don't think of efficiency. Java is optimized enough to do basic things. It only really matters when you querry a database, but for those operations, it's a fraction of a millisecond difference between this and that implementation, but making the code human readable pays much more in the long run. Good luck! – Tiberiu Dorian Moşescu Apr 21 '16 at 12:58

4 Answers4

0

You don't need to set each element to "0".

Just do this:

public static String[] wordsWithout(String[] words, String target) {
    int numberOfTargets = 0;

    for (int i = 0; i < words.length; i++){
        if ( words[i].equals(target) ) numberOfTargets++;
    }

    String[] result = new String[words.length - numberOfTargets];
    int j =0; // for indices of result
    for (int i = 0; i < words.length; i++){
        if (!words[i].equals(target) ){          
            result[j++] = words[i];
        } 
    }
    return result;
}
dryairship
  • 6,022
  • 4
  • 28
  • 54
  • Thanks a lot, guys! So much food for thought. Though I 'll keep in mind that I have to post these kind of questions in "Code review" section next time. – Alex Apr 21 '16 at 12:45
0

Looks like your code could be simplified a lot by just using an ArrayList.

public String[] wordsWithout(String[] words, String target)
{
    ArrayList<String> list = new ArrayList<String>();
    for(int i = 0; i < words.length; ++i)
    {
        if(!words[i].equals(target))
        {
            list.add(words[i]);
        }
    }
    return list.toArray(new String[0]);
}

Basically instead of calculating the size of the target array and initialising it, you use a list (which is variable in size), put in all the elements you need, and then create a new array from it.

Unrelated to that, please don't invent your own values ("0") to describe a null value - there's a dedicated keyword, null, for that.

Siguza
  • 21,155
  • 6
  • 52
  • 89
0

Use

for (String s : words) {

   if (s.equals(target))
   numberOfTargets++;
}
licklake
  • 236
  • 4
  • 15
0

This might be a bit simpler. Using the split string method allows you to create an array with each value separated by white space.

   public String[] wordsWithout(String[] words, String target) {
      String newStr = "";
      for (int i = 0; i < words.length; i++){
        if (words[i].equals(target))
          continue;
        newStr = newStr + words[i] +" ";
      }
      return newStr.split(" ");
    }
justNate
  • 1
  • 2