Garbage value when trying to remove value from dynamic array

Question

i keep getting garbage value on one of the indexes in the dynamic array when i try to remove a value which was entered by user from a list of elements in the dynamic array.

used pointer as function parameters and replaced the value to be removed with 0 and by using a counter and for loop tried to skip all the 0s but in place of zero theres a garbage value.

#include<iostream>
#include<fstream>
using namespace std;
int size = 0;
int final = 0;
int* read(ifstream& a){
    int temp;
    a.open("data(1).txt");
    while (!a.eof()){
        a >> temp;
        size++;
    }
    a.close();
    a.open("data(1).txt");
    int* arr = new int[size];
    for (int i = 0; i < size; i++)
        a >> arr[i];
    return arr;
}

int* remove(int* a,int search){
    for (int i = 0; i < size; i++){
        if (a[i] == search)
            a[i] = 0;
        else final++;
    }
    int* change = new int[final+1];
    for (int i = 0; i < size; i++){
        if (a[i] > 0){
            change[i] = a[i];
        }
        else continue;
    }
    delete[] a;
    a = nullptr;
    return change;
}

int main(){
    int* ptr = nullptr;
    int num;
    cout << "please enter the number to remove: ";
    cin >> num;
    ifstream in;
    ptr=read(in);
    ptr=remove(ptr, num);
    for (int i = 0; i < final; i++)
        cout << ptr[i] << " ";
    cout<<endl;
    system("pause");
    return 0;
}

whats inside the file? do you get the same problem when you use hardcoded input? — 463035818_is_not_an_ai, Jan 19 '23 at 15:29
[Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-i-e-while-stream-eof-cons) — 463035818_is_not_an_ai, Jan 19 '23 at 15:30
The logic of your code is flawed. You should use a debugger to see whats going on — 463035818_is_not_an_ai, Jan 19 '23 at 15:33
`if (a[i] > 0){` what about the elements in `change` where this condition is not `true`? What do you assign to them? — 463035818_is_not_an_ai, Jan 19 '23 at 15:34
While not the issue being asked about, you don't verify that you actually opened the file. — sweenish, Jan 19 '23 at 15:35
As of 2023, this is the 25th anniversary of `std::vector` being officially part of C++. Why are you not using `std::vector`? — PaulMcKenzie, Jan 19 '23 at 15:36
I don't like the `read()` function at all. It's not a very dynamic array if you have to read the file twice. Two options are much better: make the size the first value you read (an ok alternative), write an actual dynamic array class that can grow as needed (better for learning purposes), use `std::vector` (what you would actually do). — sweenish, Jan 19 '23 at 15:37
Your dependency on global variables is not good practice. And why does `remove()` appear to make a new array of one element when the value is not found, and attempt to allocate a zero sized array when it is? Generally, you shouldn't bother reallocating at all when you remove an element. You track the size and the capacity separately. — sweenish, Jan 19 '23 at 15:40
You will need to describe exactly the behavior that you expect from `remove()`. With a sample input, expected output, and actual output. This is all part of [ask]. — sweenish, Jan 19 '23 at 15:41
As said before stop using "C" style arrays, us std::vector. You should hardly see new/delete in current C++ (except in some internals of datastructures). If you need allocate memory use std::make_unique. All in all I wonder where or who you are learning C++ from. — Pepijn Kramer, Jan 19 '23 at 15:56

score 0 · Answer 1 · answered Jan 19 '23 at 16:04

There is much to discuss and critize about your code. Also your question is very unclear because we do not have your input file, nor do you tell us what the code should actually do. However, I will leave all "please write actual c++ rather than c without classes" aside and just point you to the one critical mistake:

This loop

int* change = new int[final+1];
for (int i = 0; i < size; i++){
    if (a[i] > 0){
        change[i] = a[i];
    }
    else continue;
}

And then this loop

for (int i = 0; i < final; i++)
    cout << ptr[i] << " ";

It seems like you want to copy all elements that are >0 to change. Or maybe you want to copy all, its really hard to tell, because broken code is just broken, it does not explain itself. Anyhow...

The first loop leaves all elements change[i] where a[i] <= 0 uninitialized. The values at those indices i are indeterminate. There isn't really a value you can read. Attempting to read an indeterminate value results in undefined behavior.

You are attempting to read all elements of change, but some of them are not initialized, they are indeterminate values. Hence your code has undefined behavior.

There are other situations the will bring your code into bad states, like for example search not begin found in the input array or a[i] > 0 for some i > final. Though, I don't see a possiblity for any input to your code that would not eventually invoke undefined behavior.

You sould use a debugger to see where your code is doing something unexpected.

Garbage value when trying to remove value from dynamic array

1 Answers1