I've been given a homework assignment in which I have to create a linked list, then store 25 random integers within it, after I've stored the values I have to find the sum of these 25 numbers, and then the floating point average of them. So for the most part this works. It stores the values into the list, but the member function I've created called "DataAt(int)" returning type of 'int' is only returning a value of 34 everytime. This is not what I need to assign the current sum and average outputs are:

sum = 850
avg = 34

So I need a little help with the matter because I don't seem to be able to find the problem; I've tried two different loops and and neither are wanting to work. The results should be the following based on the output from the items in the list:

sum = 1181
avg = 47.24

So anyone who can help with the matter is greatly appreciated. I've learned the main objective of the homework assignment, but I can't figure out what I'm doing wrong to retrieve data individually. For example in C# you can retrieve the data from a linked list at a specific location like you would with an array:

List<int> MyList;
int i = MyList[4]; // Valid

So I've been trying to figure it out since last night and can't seem to get it. Below is my source code and any help is appreciated in solving this matter.

// Main.cpp
#include <iostream>
#include "Random.h"
#include "LinkedList.h"

using namespace std;
using namespace LinkedList;

int main() {
    Random r;
    List<int> l;

    int sum = 0;
    float avg = 0;

    for (int i = 0; i < 25; i++) {
        cout << "Test " << i + 1 << ": " << r.Between(0, 100) << ", " << r.Solid() << "\n";
        l.InsertAtFront(r.Between(0, 100));
        sum = sum + l.DataAt(i);
    }

    avg = sum / 25;

    cout << "\n\n";

    l.Print();

    cout << "Sum: " << sum << "\nAverage: " << avg << "\n";
    cout << "Length of list: " << l.Length() << "\n";

    system("pause");
}

// Random.h
#include <stdio.h>
#include <stdlib.h>

class Random {
public:
    int Between(int min, int max);
    int Solid();
};

int Random::Between(int min, int max) { return rand() % max + min; }
int Random::Solid() { return rand(); }

// LinkedList.h
#include <iostream>

namespace LinkedList {
    template<typename T> class List;
    template<typename T>
    class Node {
        friend class List<T>;

    public:
        Node(const T &);
        T GetData() const;

    private:
        T Data;
        Node<T> *NextPointer;
    };

    template<typename T>
    Node<T>::Node(const T &info) : Data(info), NextPointer(0) {

    }

    template<typename T>
    T Node<T>::GetData() const {
        return Data;
    }

    template<typename T>
    class List {
    public:
        List();
        ~List();

        void InsertAtFront(const T &);
        void Print() const;

        bool IsEmpty() const;

        int DataAt(int Index);
        int Length();

    private:
        Node<T> *FirstPointer;
        Node<T> *LastPointer;
        Node<T> *GetNewNode(const T &);
    };

    template<typename T>
    List<T>::List() : FirstPointer(0), LastPointer(0) { }

    template<typename T>
    List<T>::~List() {
        if (~IsEmpty()) {
            cout << "Destroying nodes...\n";

            Node<T> *CurrentPointer = FirstPointer;
            Node<T> *TemporaryPointer;

            while (CurrentPointer != 0) {
                TemporaryPointer = CurrentPointer;

                cout << TemporaryPointer->Data << "\n";

                CurrentPointer = CurrentPointer->NextPointer;
                delete TemporaryPointer;
            }
        }

        cout << "All nodes destroyed...\n\n";
    }

    template<typename T>
    void List<T>::InsertAtFront(const T &value) {
        Node<T> * NewPointer = GetNewNode(value);

        if (IsEmpty())
            FirstPointer = LastPointer = NewPointer;

        else {
            NewPointer->NextPointer = FirstPointer;
            FirstPointer = NewPointer;
        }
    }

    template<typename T>
    bool List<T>::IsEmpty() const { return FirstPointer == 0; }

    template<typename T>
    Node<T> *List<T>::GetNewNode(const T &value) { return new Node<T>(value); }

    template<typename T>
    void List<T>::Print() const {
        if (IsEmpty()) {
            cout << "The list is empty...\n\n";
            return;
        }

        Node<T> *CurrentPointer = FirstPointer;

        cout << "The list is: ";

        while(CurrentPointer != 0) {
            cout << CurrentPointer->Data << " ";
            CurrentPointer = CurrentPointer->NextPointer;
        }

        cout << "\n\n";
    }

    template<typename T>
    int List<T>::DataAt(int Index) {
        Node<T> *CurrentPointer = FirstPointer;

        for (int i = 0; i < Index; i++)
            CurrentPointer = CurrentPointer->NextPointer;

        return CurrentPointer->Data;
    }

    template<typename T>
    int List<T>::Length() {
        int i = 0;
        Node<T> *CurrentPointer = FirstPointer;

        while (CurrentPointer != LastPointer) {
            CurrentPointer = CurrentPointer->NextPointer;
            i++;
        }

        if (i == 0)
            return i;

        return i + 1;
    }
}

int i = MyList[4]; // Valid

Here is my source code and maybe someone here can help me:

For some reason posting boxes are messing up on my end and I can't edit my post, it keeps highlighting areas I don't want highlighted. I don't know what's going on with it, but any help is still appreciated.

Thanks,
Jamie

If you are inserting at the front each time, DataAt(i) will always return the first element you added.
Ex:
InsertAtFront(22): 22
DataAt(0) = 22
InsertAtFront(33): 33, 22
DataAt(1) = 22
InsertAtFront(44): 44, 33, 22
DataAt(2) = 22
etc ...

At what point to do you set the random seed? I don't see any calls to srand() in the Random class, which means that the sequence of 'random' values will always be the same. You can solve this issue by adding a class integer variable seed and a static initialization function for it:

// Random.h
#include <cstdio>
#include <cstdlib>

class Random {
public:
    static void setSeed(int source) { srand(source); };
    int Between(int min, int max);
    int Solid();
};

And add a call to it in main() before using the Random objects:

    Random::setSeed(time(NULL));

Note that rand() isn't a very good random number generator to begin with, and applying modulo to it creates a bias towards smaller values. Furthermore, since only one seed is set, supposedly separate Random objects all draw from the same pseudo-random sequence. If the randomness of the values is important to you, or if you need to be able to seed the individual Random objects separately, you may want to find some other pseudo-random number generator algorithm to implement yourself, one which could be better tailored to your needs. For that matter, if you have a hardware random number generator, you may want to use that - if not directly, then as a soource of a good random seed.

I would also add that you should use the newer forms of the include headers, such as <cstdlib> and <ctime>, rather than the C-style <stdlib.h> and <time.h>.

As for using the array syntax, you can simply replace DataAt() with the following:

template<typename T>
T List<T>::operator[](int Index)
{
    Node<T> *CurrentPointer = FirstPointer;
    for (int i = 0; i < Index; i++)
        CurrentPointer = CurrentPointer->NextPointer;
    return CurrentPointer->Data;
}

Only the function signature is changed; the rest is the same as you had it before, but now you can use

sum += l[i];

instead of

sum += l.DataAt(i);

HTH.

Edited 4 Years Ago by Schol-R-LEA

Well I've known about operator overloading for a while now so thank you for reminding me how to do it, as for the random numbers; it doesn't have to be perfectly random, just random enough that it works. The problem is probably as histrungalot stated; I need to find another way to insert the nodes into the list because it's doing exactly what he demonstrated.

You can do it the way you are just reference the DataAt(0) to see the random number you just added. Not an elegant solution but it will get you started.

Edited 4 Years Ago by histrungalot: Fix

So in pseudocode it would be something like:

int List<T>::DataAt(int Index) {
    Node<T> *CurrentPointer = LastPointer;

    for (int i = 0; i < Index; i++)
        CurrentPointer = CurrentPointer->NextPointer;

    return CurrentPointer->Data;
}

And of course all my pseudocode has to be syntactically correct lol sorry; anyways that's from my understanding is that I'm putting the list in backwards and it's only reading from the front because that index will always be there?

Why not two loops

// etc...
int main() {
    Random    r;
    List<int> l;

    int sum(0);
    float avg(0);

    // ------------------------------------
    // Create the list of random values
    for (int i = 0; i < 25; i++) 
        l.InsertAtFront(r.Between(0, 100));

    // ------------------------------------
    // Why not a second loop
    for (int i = 0; i < 25; i++) 
       sum += l.DataAt(i);

    // ------------------------------------
    // Need to make 25 a float to get the 
    // decimal
    avg = sum / 25.0f;
// etc...
This article has been dead for over six months. Start a new discussion instead.