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?

Improve this question
2
  • 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. Commented Apr 21, 2016 at 12:25
  • You'll find the answer here : stackoverflow.com/questions/7940337/… 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! Commented Apr 21, 2016 at 12:58

4 Answers 4

Reset to default
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;
}
Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

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.
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.

Improve this answer

3 Comments

Thanks, got it! Is there any difference to use ++i or i++ in for loop?
@Alex Nah, it's just a personal preference.
@Alex in this case not, but have a look here: stackoverflow.com/questions/2315705/…
0

Use 

for (String s : words) {

   if (s.equals(target))
   numberOfTargets++;
}
Improve this answer

Comments

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(" ");
    }
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.