0
#include<iostream>
using namespace std;
template<typename T>
class List
{
public:
    T *values;
    int capacity;
    int counter;
public:
    List()
    {
        values = NULL;
        capacity = 0;
        counter = 0;
    }
    List(int cap)
    {
        capacity = cap;
        values = new T[cap];
        counter = 0;
    }
    bool insert(T item)
    {
        if (isFull() == false)
        {
            values[counter] = item;
            counter++;
            return true;
        }
        return false;
    }
    bool insertAt(T item, int index)
    {
        
        if (isFull() == false && index < counter)
        {
            capacity++;
            for (int i = capacity; i > index; i--)
                values[i] = values[i - 1];
            
            values[index] = item;
            
            return true;
        }
        return false;
    }
    bool isFull()
    {
        if (counter == capacity)
        {
            return true;
        }
        return false;
    }
    void print()
    {
        for (int i = 0; i < capacity; i++)
        {
            cout << values[i] << " ";
        }
    }
    
};
int main()
{
    List<int> obj1(5);
    obj1.insert(1); //0
    obj1.insert(2); //1
    obj1.insert(3); //2
    obj1.insert(4); //3
    
    obj1.insertAt(3, 1);
    obj1.values[1];
    obj1.print();
    
}

Kindly look into this program I have to insert an element at given position. But when I run this program I am getting garbage at the end element of an array. Kindly check and let me know where is the problem? Please check the insertAt function I think this function has some logical error. I have added the main function when I call print function it give garbage at the last index

user4581301
  • 33,082
  • 7
  • 33
  • 54
  • You should add a simple `main` function that exposes the error you are hunting so people can experiment with the code and be able to see exactly (within allowances for Undefined Behaviour) what you see – user4581301 Sep 20 '22 at 17:44
  • @user4581301 I have edited the question kindly check and let me know where is the problem – Mudassir Waheed Sep 20 '22 at 17:48
  • Thank you. Unrelated note: The default constructor doesn't appear to be very useful. Since there is no way to extend the size of the `List` you are stuck, always and forever, with a `List` of size zero. – user4581301 Sep 20 '22 at 17:49
  • A question. On a closer read, I see `insertAt` increased the `capacity` member, not `counter`. Is this intentional? Is the goal to increase the size of the array, if so, none of the other infrastructure necessary to allocate a larger array is present. If not, this is possibly where your bug lies. – user4581301 Sep 20 '22 at 17:52
  • 1
    Adding this because it seems to be omitted from most programming courses: Take advantage of whatever debugger comes with your development environment. With a debugger you can step through the program line byline and see exactly what the program does as it does it. As soon as you see the program do something you didn't expect, such as store the wrong value or take the wrong path, you've found a mistake. Either it is a bug in the code or a error in your expectations; regardless it needs to be fixed. – user4581301 Sep 20 '22 at 18:00

2 Answers2

1

There are a few issues with your insertAt function:

  1. You need to increase counter, not capacity. Your capacity is fixed at construction time. It cannot change without allocating a new underlying array and copying the elements over.
  2. You also should only run your loop from counter to index rather than starting at capacity, since the elements between counter and capacity will be uninitialized assuming T is a trivial type.
  3. You shouldn't set values[index] = item until after the loop has finished moving all of the existing items.
  4. You should return true after the loop, and return false outside the if block
bool insertAt(T item, int index)
{
    if (!isFull() && index < counter)
    {
        for (int i = counter; i > index; i--) {
            values[i] = values[i - 1];    
        }
        values[index] = item;
        counter++;
        return true;
    }
    return false;
}

Note: You have a similar mistake in your print function. You're printing all of the values up to capacity, but the ones between counter and capacity are uninitialized. That loop should also terminate at counter.

Demo


It's also worth pointing out that you never delete[] values, so that memory will leak when your list object dies. You also haven't followed the rule of three though, so if you add a destructor that delete[]s values you'll also need a copy-constructor and copy-assignment operator that copy the underlying array so that multiple list objects don't try to delete[] the same array.

Miles Budnek
  • 28,216
  • 2
  • 35
  • 52
  • I have folllowed what you said but now it is inserting at the right position but in the last index value is 4 not 5 – Mudassir Waheed Sep 20 '22 at 18:00
  • @OmegaCoding Note: I made a mistake; see my latest edit. The extra `- 1` is not needed and the loop should not be inclusive of `index`, since you're moving from items from `values[i - 1]` to `values[i]`. – Miles Budnek Sep 20 '22 at 18:02
  • I have checked the updated one still las t element is missing:(\ – Mudassir Waheed Sep 20 '22 at 18:10
  • @OmegaCoding It [works fine for me](http://coliru.stacked-crooked.com/a/163e5a345ab1f5c5). There was also a bug in your `print` function that I added a note about. – Miles Budnek Sep 20 '22 at 18:14
-1
bool insertAt(T item, int index)
    {
        
    
        if (!isFull() && index < counter)
        {
            for (int i = counter; i > index; i--) {
                values[i] = values[i - 1];
            }
            values[index] = item;
            counter++;
            return true;
        }
        return false;
    }

void print()
    {
        for (int i = 0; i < counter; i++)
        {
            cout << values[i] << " ";
        }
    }

I have corrected these two function. Try these functions instead of the one you implemented