3

I've scoured a couple of the SOF threads but can't seem to find the answer I'm looking for. Most of them provide an answer with code that's beyond the scope of what I have learned thus far.

I've tried quite a few different things and can't get this to work the way I need it to.

The program is supposed to take the given array, read it, find the given toRemove item, and re-print the array without the toRemove item.

I believe my issue is within the removeFromArray method

public static void main(String[] args) 
{

    String[] test = {"this", "is", "the", "example", "of", "the", "call"};
    String[] result = removeFromArray(test, "the");
    System.out.println(Arrays.toString(result));
}

public static String[] removeFromArray(String[] arr, String toRemove)
{
    int newLength = 0;
    for(int i = 0; i < arr.length; i++)
    {    
        if(arr[i].contains(toRemove))
        {
            newLength++;
        }
    }
    String[] result = new String[arr.length-newLength];
    for(int i = 0; i < (result.length); i++)
    {
        if(arr[i].contains(toRemove))
        {

        }
        else
        {
            result[i] = arr[i];
        }
    }
    return result;
}

This is an assignment in my java class and we have not learned Lists (one of the answers I stumbled upon in my googling) yet so that is not an option for me.

As it is now, it should be outputting: [this, is, example, of, call]

Currently it is outputting: [this, is, null, example, of]

Any and all help will be much appreciated!

Improve this question
1
  • You probably want to use .equals instead of .contains. If you use .contains, your method would also remove "the dog", "absinthe", etc. from the array. Commented Nov 2, 2017 at 7:01

5 Answers 5

Reset to default
5

You need 2 indices in the second loop, since you are iterating over two arrays (the input array and the output array) having different lengths.

Besides, newLength is a confusing name, since it doesn't contain the new length. It contains the difference between the input array length and the output array length. You can change its value to match its name.

int newLength = arr.length;
for(int i = 0; i < arr.length; i++)
{    
    if(arr[i].contains(toRemove))
    {
        newLength--;
    }
}
String[] result = new String[newLength];
int count = 0; // count tracks the current index of the output array
for(int i = 0; i < arr.length; i++) // i tracks the current index of the input array
{
    if(!arr[i].contains(toRemove)) {
        result[count] = arr[i]; 
        count++;
    }
}
return result;
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

0

There's the error that @Eran pointed out in your code, which can solve your problem. But I'm going to discuss another approach.

For now, you're first iterating over the entire array to find the number of occurrences to remove, and then, you're iterating over the array to remove them. Why don't you just iterate over the array, just to remove them. (I know, your first loop is helping you to determine the size of the output array, but you don't need that if you use some List like ArrayList etc.)

List<String> resultList = new ArrayList<String>();
for(int i = 0; i < arr.length; i++)
{
    if(!arr[i].contains(toRemove))
    {
        resultList.add(arr[i]);
    }
}

And you can return the resultList, but if you really need to return an array, you can convert the resultList to an array like this:

String [] resultArray = resultList.toArray(new String[resultList.size()]);

And then return this array. See this approach live here on ideone.

Improve this answer

6 Comments

Same difference really, though this is a simpler approach. Depending on the relative time reads and writes take on a given machine, OPs approach could possibly be faster.
I think not. Because in OPs approach, there's a contains operation in two loops. The overall time taken is (2 * N * time taken by contains), N being the input array length. But in this approach, it is (N * time taken by contains) + (N * time taken to copy), and contains operation is way costly than just a mere copy operation.
Actually yeah you're right. And actually OP probably shouldn't be using contains anyway, but rather equals
Note that time taken to copy is actually O(N), not truly N, as there is some cost for periodically resizing the ArrayList.
Yes, of course, we can't ignore the time of resizing the ArrayList.
|
0

Try this Java8 version

    List<String> test = Arrays.asList("this", "is", "the", "example", "of", "the", "call");

    test.stream()
        .filter(string -> !string.equals("the"))
        .collect(Collectors.toList())
        .forEach(System.out::println);
Improve this answer

Comments

0

You can use Java Stream instead, it will give you the expected result and also your code will be clearer and really smaller.

See the method below I wrote that solves your problem.

public static String[] removeFromArray(String[] arr, String toRemove) {
    return Arrays.stream(arr)
      .filter(obj -> !obj.equals(toRemove))
      .toArray(String[]::new);
}

If you're unfamiliar with java Stream, please see the doc here

Improve this answer

Comments

0

The following code removes all occurrences of the provided string.

Note that I have added few lines for validate the input, because if we pass a null array to your program, it would fail. You should always validate the input in the code.

public static String[] removeFromArray(String[] arr, String toRemove) {

    // It is important to validate the input
    if (arr == null) {
        throw new IllegalArgumentException("Invalid input ! Please try again.");
    }

    // Count the occurrences of toRemove string.
    // Use Objects.equals in case array elements or toRemove is null.
    int counter = 0;
    for (int i = 0; i < arr.length; i++) {
        if (Objects.equals(arr[i], toRemove)) {
            counter++;
        }
    }

    // We don't need any extra space in the new array
    String[] result = new String[arr.length - counter]; 
    int resultIndex = 0; 

    for (int i = 0; i < arr.length; i++) {
        if (!Objects.equals(arr[i], toRemove)) {
            result[resultIndex] = arr[i];
            resultIndex++;
        }
    }

    return result;
}
Improve this answer

5 Comments

Objects.isNull is intended primarily as a filter predicate (e.g. stream.filter(Objects::isNull).count(). Better to use arr == null and toRemove == null as Objects.isNull just performs == null.
Also, there's no reason to throw an error when arr.length == 0. After all, Collection.remove works perfectly well when the collection is empty.
The caller may actually want to remove null elements by calling removeFromArray(arr, null). Better to use Objects.equals(arr[i], toRemove) to tolerate null values in the array and toRemove.
I have modified the answer accordingly. Thanks for the suggestions. Although I just had a look at the source code of isNull method. It simply checks if the element is null, nothing fancy.
Thanks! I put in the Objects.equals check I was talking about.

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.