Passing Arrays and Vectors

[coolbeanbob]

Hello,

I am having trouble implementing the load function below. It compiles, but I get garbage in the array and 0's in the vector.

The reason I created load() was to clean up main(). Do I need to open data.txt in main and then call a function to load the data structures?

#include "utility.h"
#include "lnklist.h"
#include <vector>


int main()
{
    void printArray(int array[]);       //need to combine
    void printVector(vector<int> &v);   //these two functions

    void load(int array[], vector<int> &v);


    int array[30];                              //declare an array of type int with size 30.
    vector<int> vectorOne(300);                  //declare a vector of type int with size 31.

/*    int number;
    int index = 0;

//=======================================================
    ifstream in_stream;
    in_stream.open("data.txt");
    if(in_stream.fail( ))
    {
      cout << "unable to find input file";      //displays error message if file is not found
      exit(1);                                  //exits the program if file is not found
    }//end if in_stream.fail( )
//=======================================================

    while (in_stream >> number)     //in_stream >> number returns a Boolean value of true if the operation has been successful.
    {
        array[index] = number;

        vectorOne.at(index) = number;

        index++;
    }//end while in_stream >> number.
*/

    printArray(array);
    printVector(vectorOne);
    load(array, vectorOne);

    cout << "Done!" << endl;

    return 0;
}

void load(int array[], vector<int> &v){
    int number;
    int index = 0;

//=======================================================
    ifstream in_stream;
    in_stream.open("data.txt");
    if(in_stream.fail( ))
    {
      cout << "unable to find input file";      //displays error message if file is not found
      exit(1);                                  //exits the program if file is not found
    }//end if in_stream.fail( )
//=======================================================

    while (in_stream >> number)     //in_stream >> number returns a Boolean value of true if the operation has been successful.
    {
        array[index] = number;

        v.at(index) = number;

        index++;
    }//end while in_stream >> number.
}

void printArray(int array[]){
    for (int i = 0; i < 30; i++)
    {
        cout << "Array:  " << array[i] << endl;
    }
}//function used to print data structures. See implementation below.


//Combine this with printArray using overloaded function
void printVector(vector<int> &v){
    for (int i = 0; i < 30; i++)
    {
        cout << "Vector: " << v[i] << endl;
    }
}//end printVector

[Schol-R-LEA]

The comment here is incorrect, which may shed light on how the loop is failing as well:

while (in_stream >> number)     //in_stream >> number returns a Boolean value of true if the operation has been successful.

The >> operators, as a rule, return a reference to an istream not a boolean value per se. Mind you, a non-null pointer should be 'truthy' (that is, it would be treated as a true value), so I would expect the while() loop to continue indefinitely.

A more typical idiom for this would be

for (int index = 0; in_stream.good(); index++) 
    {
        in_stream >> number;
        array[index] = number;
        v.at(index) = number;
    }
    // check to see if it stopped because of an I/O error
    if (in_stream.fail())
    {
        //put error handling code here
    }

Well, OK, it would normally done with a while() rather than a for() , but in this case it makes for a more compact and integral expression.

[ravenous]

You seem to be a little confused about defining and using functions in C++. Currently your program looks like this:

int main()
{
    void printArray(int array[]);
    void printVector(vector<int> &v);
    void load(int array[], vector<int> &v);
 
    int array[30];
    vector<int> vectorOne(300);
    
    printArray(array);
    printVector(vectorOne);
    load(array, vectorOne);
 
    return 0;
}

This doesn't do what you think it's doing. What you should have is:

void printArray(int array[]);
void printVector(vector<int> &v);
void load(int array[], vector<int> &v);
 
int main()
{
    int array[30];
    vector<int> vectorOne(300);
    
    printArray(array);
    printVector(vectorOne);
    load(array, vectorOne);
 
    return 0;
}

Note that the functions are now declared outside main . OK, on to your actual problem. The reason why you get zeros and garbage is because you're calling your "print" functions before you're adding stuff to the array and vector. If you load values into the array and vector before printing then you might get better results (as well as doing the things that Schoil-R-LEA said).

You're also committing a number of other C++ sins, but we'll deal with those another day :o)

[mike_2000_17]

Disregard what Scholl-R-LEA just said. This loop is correct:

while (in_stream >> number)

In fact, the above is the idiomatic way of reading from a file and it is correct. The operator >> does indeed return a reference to a istream, such that it can be chained. However, the istream type is implicitly convertible to a (void*) type which is only NULL (which then converts to "false") if the istream object is no longer in good condition for input (e.g. hasn't reached the end-of-file marker for example).

The reason you get garbage in the "array" is because you call the print function before you call the load function, so "array" is still uninitialized when you print it.

The reason you get 0 in the vector is because, again, you call the print function before you call the load function, so the vector is filled with zeros because that's what you construct it with (default fill-value is zero).

Just put line 42 before line 40-41 and you should get what you expect.

However, there are some additional issues.

First, your function prototypes should not be in the body of the main() function. I'm not sure why your compiler accepts it or if it should (but I don't think it should). You should declare your functions outside the main() function, that is, before it. So, you should put lines 8 to 11 before line 6.

Second, your code is not safe. What if there are more than 30 numbers in the input file? You should have some way of dealing with that. Either limit the number of inputs to 30, or grow the array if necessary. It's easier to limit the number of inputs to 30, as so:

while ( (in_stream >> number) && (index < 30) )

Finally, you have too many magic numbers in your code. Magic numbers are things like the "30" in your code. What if you decide to change it to 40? You will have to change it everywhere. You should have a global constant for that:

const unsigned int numberOfInts = 30;

And then replace every occurrence of "30" with this constant variable.

Also, normally, a "printArray" function like yours should also pass the number of elements in the array, and it is more idiomatic to pass the array as a pointer to the first element:

void printArray(int* array, unsigned int size) {
    for (int i = 0; i < size; i++)
    {
        cout << "Array:  " << array[i] << endl;
    }
}//function used to print data structures. See implementation below.

Your printVector function should also do the same, but since the size is accessible from the vector class, you just need to do:

//Combine this with printArray using overloaded function
void printVector(vector<int> &v){
    for (int i = 0; i < v.size(); i++) //notice v.size() here.
    {
        cout << "Vector: " << v[i] << endl;
    }
}//end printVector

As for overloading, well you just need to give the same name to both functions, et voilà!

[coolbeanbob]

Thanks guys. This is all very helpful. I really appreciate it.

[coolbeanbob]

Is it better practice to use an overloaded function vs passing two data types to the same function at once?

Example:

Above I have

load(array, vectorOne);

Would it be better to say

load(array);
load(vectorOne);

[mike_2000_17]

>>Is it better practice to use an overloaded function vs passing two data types to the same function at once?

Overloading the function. Otherwise, you need an overload for every possible pair of data types or multiple combinations, that's not scalable.

I think a more pertinent question is: Is it good practice to have two data structures that contain the same information? NO. If neither of two data structures are good for your purpose, make your own, but don't keep two data structures in parallel.

[coolbeanbob]

>>Is it better practice to use an overloaded function vs passing two data types to the same function at once?

Overloading the function. Otherwise, you need an overload for every possible pair of data types or multiple combinations, that's not scalable.

I think a more pertinent question is: Is it good practice to have two data structures that contain the same information? NO. If neither of two data structures are good for your purpose, make your own, but don't keep two data structures in parallel.

Sorry, I realize having two parallel data structures is a bad idea. This is an assignment. Actually, I'm working on adding a third data structure (linked list) to the mix.

I understand your comment about the scalability. It seems like I am re-typing a lot of the same code just in order to create the overloaded constructor.

Does the below format make the most sense for scalability?

I need to check the user's response against all three data structures. At first, I started to create one function that passed all three data structures, then I remembered this thread.

int main()
{
    void checkResponse(response, array);
    void checkResponse(response, vectorOne);
    void checkResponse(response, MyList);
}

void checkResponse(int response, int array[])
{
    if(response <= sizeof(array))
    {
        cout << "ok" << endl;
    }
}


void checkResponse(int response, vector<int> &v)
{
    if(sizeof.v <= 30)
    {
        cout << "ok" << endl;
    }
}

void checkResponse(int response, <int>List &m)
{
    if(sizeof.m <= 30)
    {
        cout << "ok" << endl;
    }    
}

The above code is what I believe you are saying is the correct way.

Below is what I intuitively want to write...

int main()
{
    void checkResponse(response, array, vectorOne);
}

checkResponse(int response, int array[], vector<int> &v)
{
    //check all lengths here
}

[mike_2000_17]

Well, you sort of have it correct (with those three functions).

But, in general, the rule is if two functions are the same, they should be one, if two functions are different they should be two. However, with your limited knowledge of C++, you have limited means to implement this. The usually way to deal with this is like the STL algorithms do it (like std::copy). Usually all kinds of containers (arrays, vectors, lists, etc.) can, in most cases, be abstracted as an iterator to the beginning and end (one elements past the end) of the range. That usually allows you to write one function that can deal with any kind of container with just one function. But you need to know about templates. So, be patient with the whole "what's the best way to do this.." line of questioning, it may lead you beyond what you can currently grasp.

[NathanOliver]

If you are checking that data inside the containers against the supplied response then you might want to think about using templates. Here is a simple way to check through containers using iterators.

template<typename ForwardIterator>
void CheckResponse(int response, ForwardIterator first, ForwardIterator last) // last should be one past the end
{
    while (first++ != last)
    {
        if (response == *first)
            cout << "found";
    }
}

// now you can use this function like
const int listSize = 20
int list1[listSize];
vector<int> list2(listSize);
list<int> list3(listSize);
int response;

CheckResponse(response, list1, list1 + listSize);  // array going one past the end
CheckResponse(response, list2.begin(), list2.end()); // vector, end goes one past the end
CheckResponse(response, list3.begin(), list3.end()); // list, end goes one past the end

[coolbeanbob]

Sorry this code is so messy. I am trying to get the length of the array. Line 31 is commented out, but it gave me the correct value of 30. I have the same code on line 114 (I don't want to use line 31), but I get the value of 1.

I thought I might need to pass the array by reference, but it looks like arrays are by default passed by reference.

#include "utility.h"
#include "lnklist.h"
#include <vector>


    void load(int array[]);
    void load(vector<int> &v);
    void print(int array[]);
    void print(vector<int> &v);
    void checkResponse(int response, int array[], vector<int> &v);
    //void checkResponse(int response, vector<int> &v);

int main()
{
    int response;
    int array[30];                              //declare an array of type int with size 30.
    vector<int> vectorOne(30);                  //declare a vector of type int with size 31.
    List <int>MyList;

    load(array);
    load(vectorOne);
    print(array);
    print(vectorOne);


/*    do
     {
        cout << "User! Please enter the position of the item you wish to delete!" << endl;
        cin >> response;

        cout << "array " << sizeof(array)/sizeof( array[0] ) << endl;
        cout << "vector " << vectorOne.size() << endl;

     } while(response > sizeof(array)/sizeof( array[0] ) && response > vectorOne.size());
*/


    checkResponse(response, array, vectorOne);
    //checkResponse(response, vectorOne);
    //checkResponse(response, MyList);

    return 0;
}//end main()

void load(int array[])
{
    int number;
    int index = 0;

    ifstream in_stream;
    in_stream.open("data.txt");
    if(in_stream.fail( ))
    {
      cout << "unable to find input file";      //displays error message if file is not found
      exit(1);                                  //exits the program if file is not found
    }//end if in_stream.fail( )

    while (in_stream >> number)     //in_stream >> number returns a Boolean value of true if the operation has been successful.
    {
        array[index] = number;
        index++;
    }//end while in_stream >> number.
}//end load()

void load(vector<int> &v)
{
    int number;
    int index = 0;

    ifstream in_stream;
    in_stream.open("data.txt");
    if(in_stream.fail( ))
    {
      cout << "unable to find input file";      //displays error message if file is not found
      exit(1);                                  //exits the program if file is not found
    }//end if in_stream.fail( )

    while (in_stream >> number)     //in_stream >> number returns a Boolean value of true if the operation has been successful.
    {
        //array[index] = number;

        v.at(index) = number;

        index++;
    }//end while in_stream >> number.
}//end load()




void print(int array[])
{
    for (int i = 0; i < 30; i++)
    {
        cout << "Array:  " << array[i] << endl;
    }
}//end print(array)

void print(vector<int> &v)
{
    for (int i = 0; i < 30; i++)
    {
        cout << "Vector: " << v[i] << endl;
    }
}//end print(vector)

void checkResponse(int response, int array[], vector<int> &v)
{
    do
     {
        cout << "User! Please enter the position of the item you wish to delete!" << endl;
        cin >> response;

        cout << "array " << sizeof(array)/sizeof( array[0] ) << endl;
        cout << "vector " << v.size() << endl;

     } while(response >= sizeof(array)/sizeof( array[0] ) && response >= v.size());
}



/*
void checkResponse(int response, vector<int> &v)
{
    if(v.size() <= 30)
    {
        cout << "ok" << endl;
    }
}

void checkResponse(int response, List)
{
    if(List <= 30)
    {
        cout << "ok" << endl;
    }
}
*/

[NathanOliver]

There is no way to get the size of an array after it has been passed to a function. here is a good explanation.

[raptr_dflo]

At this point, you're learning the nuances of dealing with an array vs. a std::vector. One benefit (out of several) of using a vector, is that it maintains its own size for you. With the array, as you're discovering, along with the array itself (or pointer to element zero, the two are essentially interchangeable), you need to pass some indication of how many elements are in it -- either a numeric size, which is the more accepted idiom, or a pointer to the last element or one-past-the-last-element, if that provides a more natural expression of what you're trying to accomplish.