CODEWITHSUNDEEP
c++functionstructure
jps_dot_dev
jps_dot_dev

Reputation: 125

Function not saving to structure

Maybe I'm burnt out but I cannot figure out why every time I run this code the getInfo function will not write the cin data to the MovieData structure.

#include <iostream>
#include <string>
using namespace std;

struct MovieData {
    string Title, Director, Year_Released, RunningTime;
};

MovieData film1;
MovieData film2;

void getInfo(MovieData something)
{
    cout << "Please enter...\n";
    cout << "Title: ";
    cin >> something.Title;
    cout << "Director: ";
    cin >> something.Director;
    cout << "Year Released: ";
    cin >> something.Year_Released;
    cout << "Running Time: ";
    cin >> something.RunningTime;
}

void showInfo(MovieData something1)
{
    cout << something1.Title << endl;
    cout << something1.Director << endl;
    cout << something1.Year_Released << endl;
    cout << something1.RunningTime << endl;
    cin.get();
    cin.get();
}



int main()
{
    getInfo(film1);
    showInfo(film1);
    cin.get();
    cin.get();
    return 0;
}

I don't know whether this is some kind of global/local issue it seems to only work if I manually have every get info line in the main function.

Upvotes: 2

Views: 1445

Answers (3)

Red Alert
Red Alert

Reputation: 3816

The reason what you've posted doesn't do what you expect is because you pass something by value. Essentially, film1 is copied into something, then you modify that copy, then the copy is destroyed at the end of the function. film1 is never touched.

You can get around that by passing by reference, as toth mentions, however, if the goal of getInfo is to populate a MovieData, why not return it instead of modifying your input parameter? It makes the function's signature more intuitive. For example: 

MovieData getInfo()
{
    MovieData something
    cout << "Please enter...\n";
    cout << "Title: ";
    cin >> something.Title;
    cout << "Director: ";
    cin >> something.Director;
    cout << "Year Released: ";
    cin >> something.Year_Released;
    cout << "Running Time: ";
    cin >> something.RunningTime;

    return something;
}

Then in main, you can

MovieData film1 = getInfo();
showInfo(film1);

This way, no one can pass an already popuplated MovieData into your function and have it clobbered.

Upvotes: 2

Chris Drew
Chris Drew

Reputation: 15334

You are passing MovieData to getInfo by value so you are changing a local copy within the function and the original variable does not see the changes.

You could fix it by taking a reference instead (MovieData&) but the more idiomatic C++ way would be to use a return type:

MovieData getInfo()
{
    MovieData something;
    cout << "Please enter...\n";
    cout << "Title: ";
    cin >> something.Title;
    cout << "Director: ";
    cin >> something.Director;
    cout << "Year Released: ";
    cin >> something.Year_Released;
    cout << "Running Time: ";
    cin >> something.RunningTime;
    return something;
}

Off-topic: It is generally best to avoid global variables like film1 if you can. In this case you can define the variable within the main function:

int main() {
    MovieData film = getInfo();
}

Upvotes: 0

toth
toth

Reputation: 2552

If you change the declaration of getInfo to

void getInfo(MovieData& something)

(note the added &) it should work. The issue is that you are passing something by value and you want to pass it by reference.

When you call getInfo with your current declaration, the something it gets is only a copy of argument you passed in. That copy is modified by the function, but the original object stays the same. By adding changing the type of the parameter to MovieData& you are passing the argument by reference, and modifications inside the function will be reflected in the original object.

In general, in C++ if you need to modify function arguments you should pass them by reference, not value.

Upvotes: 6

Related Questions