Click here to Skip to main content
15,913,159 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
Hi,
I was trying to implement a contiguous list. Now, this is compiling well, but on trying this small insert method of list, the entries are not inserted into the list. Here;s an example from the main program:
Main.cpp
#include <iostream>
#include "List.h"
#include "Random.h"
#include "Timer.h"
#include "Key.h"
using namespace std;
int main()
{
    int n;
    List<int> the_list;
    cout<<"Enter the number of records to be stored. "<<endl;
    cin>>n;
    for(int i=1;i<n;i=i+2)//Enter only odd values upo n
    {
    the_list.insert(i,i);//(position, List_entry &x)
    }
    cout<<the_list.size();
    return 0;
}



Output: Enter the number of records to be stored:
5
0 (which should have been 5)

List.h
const int maxlist=1000;
enum Error_code
{
    success,
    overflow,
    underflow,
    range_error
};
template <class List_entry>
class List
{
    public:
    //List();

    List():count(0)
    {}

    bool empty()
    {
        return count==0;
    }
    bool full()
    {
         return count==maxlist;
    }
    int size()
    {
        return count;
    }
    Error_code insert(int position, const List_entry &x)
    {
        if(full()) 
        {
            return overflow;
        }
        if(position<0 || position>count){ return range_error;}
        for(int i=count-1;i>=position;i++)
        {
            entry[i+1]=entry[i]; 
        }
        entry[position]=x;
        count++;
        return success;
    }
    Error_code retrieve(int position, List_entry &x)const
    {
        if(empty()) return underflow;
        if(position<0 || position>count) return range_error;
        x=entry[position];
        return success;
    }

    Error_code replace(int position, const List_entry &x)
    {
        if(empty()) return underflow;
        if(position<0 || position>count) return range_error;
        entry[position]=x;
        return success;
    }
    Error_code remove(int position, List_entry &x)
    {
        if(empty()) return underflow;
        if(position>0 || position>=count) return range_error;
        for(int i=position;i<=count-1;i++)
        entry[i+1]=entry[i];
        count--;
        return success;
    }

    void traverse(void (*visit)(List_entry &x))//*visit is the      pointer to function which performs operation on list
    {
        for(int i=0;i<count;i++)
        (*visit)(entry[i]);
    }

    protected:
    int count;
    List_entry entry[maxlist];
};


What's going wrong here?

Edited for readability
-Emilio-
Posted
Updated 1-Jan-11 21:15pm
v2
Comments
Sergey Alexandrovich Kryukov 1-Jan-11 22:56pm    
What, you don't have a debugger, or need someone to tell you how to use it?
optimus_prime1 1-Jan-11 23:13pm    
How? Is it available in Code::Blocks because I am trying applications on console.
Emilio Garavaglia 2-Jan-11 3:38am    
Sure: set a breakpoint at the beginning of insert (click on the left margin: a red bullet should appear), then do "Debug / Start" and when the execution stops (you should see a yellow arrow), step the lines and watch the variables.

Before to actually "insert", you have to shift what is already inside.
But there is a bug here
C++
for(int i=count-1;i>=position;i++)
{
    entry[i+1]=entry[i];
}

To shift "one advance" you must walk backwardly (otherwise you'll overwrite your foots). You start at count-1 (last element) to go down to the insertion point but ... you step i++, but it should have been i--.


-Suggestion: unless specifically required, use ++i and --i rather than i++ and i--. Until you work with integers that's not a problem, but when in future you'll move to generic code and iterators, prefix increment are more efficient, since they don't require a copy and destroy.



-Emprovement: Your code is very basic, and it is far from being efficient:
Try finding a way to have a "logic" list where the sequence is not necessarily the same as the physical one. You can this way avoid all the "shifts".
 
Share this answer
 
Based on your prior posts, you will learn a lot from Large Scale C++ Design by Lakos. His book would have answered several of the questions you have asked on codeproject.
 
Share this answer
 
Comments
Sergey Alexandrovich Kryukov 2-Jan-11 13:43pm    
Who voted "2"? The one who is too lazy to read a book?
Not a very practical answer to get a quick solution, but does not deserve "2".
I don't have to eat all the egg to tell it's bad, right?

So, in your test sample, you always insert at the index less than count; which returns range_error -- quite visible with unarmed eye, even without debugger. Is that all?

Well, I don't want to examine all the code: it does not deserve it; and the problem is too trivial. But I think you ought to know what's bad in your code. Nearly everything:


  1. maxList is hard-coded; you could use limits.h, but this is not so important because of next item.
  2. maxList and constant-size array is bad: you always occupy whole array in memory; it is not used if count is less; at the same time the capacity may be not enough.
    You could use heap memory instead; alternatively, you could use linked list which is better with random-position insertion.
  3. Returning Error_code if fantastic, wasteful anachronism: you should better use exceptions instead.
  4. Why all those irrelevant include lines?
  5. What you marked protected should be private (deriving other classes from this one is pointless).
  6. traverse is not flexible at all; you should have implemented iterator pattern instead; which is easy enough to do.
  7. You code is not readable due to poor formatting.
  8. Maybe, more... buy that's enough.


A school assignment? Hm...
 
Share this answer
 
Comments
Emilio Garavaglia 2-Jan-11 3:09am    
This answer is technically perfect, but practically useless.
It is clear the OP is just starting programming from yesterday or such.
Things like "exception", "iterator" and "flexibility" at his level are probably just "buzzwords".
His code doesn't need to be perfect. It just need to "work". The important thing -at this level- are not "make a large scale project reusable module" but "understanding the mechanics that's inside and beyond a list".

Do you remember when you were a "child"?
Sergey Alexandrovich Kryukov 2-Jan-11 12:56pm    
You arguments about a beginner are questionable. How do you think we should teach the beginners. By baby steps? The we're helping growing engineers who remain babies throughout their entire carriers.

My approach is different; and it works. The key is to offer adequate, small assignment at first, but requirements should be serious from the very beginning.

Also, I know how good students work. I rather compare with them.
And I do remember my own child steps in programming or not. Nobody told me "good job" until it was really good.
Emilio Garavaglia 3-Jan-11 8:41am    
First of all: I didn't voted. So please, before flame, calm down and think a while: we are not the one and only persons looking at this page.

About teaching: I've also been a teacher and I knopw there are two way to teach biking: with support wheels and without. The second method can be faster, but you must be prepared with some patches!

The good teacher use support wheels, and attempt to guess the best moment to remove them.
Sergey Alexandrovich Kryukov 3-Jan-11 13:32pm    
Emilio, Sorry for wrong assumption -- my bad. I will of course remove the blame.

About teaching I'm not quite agree with you: I prefer without support wheels. Consider different mechanics which has nothing to do the the real-life -- pretty much to my point.
At the same time, in real life I do a lot more baby steps then I would like to. Would you see how many attempts I have already done to help optimus_prime? Once you started, it's hard to stop, even though I want to give up -- too much time wasted.
Emilio Garavaglia 3-Jan-11 14:55pm    
My impression is that he's following an incoherent path. He's not yet "stable" with trivial algorithms (++ and -- in a down loop), needs to experiment with lists (so we cannot say "use STL": he will never view the details... But he's actually managing array contents a-la C) and at the same time he's using templates!
He seems to me to be lost because of a misguided too long step ... a long time ago! (or ... may be because he's too hurried to move forward).

He needs to stop a while and reorder his mind, rather then for(;;)Retry();

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900