Help with passing arrays

Started by
4 comments, last by Zahlman 17 years, 11 months ago
I am trying to pass arrays by reference but it keeps giving me this error: Prog16.cpp:37: cannot convert `std::string' to `std::string*' for argument `1' to `void AddStudent(std::string*, int*, int)' shortened code as follows: " #include <iostream> #include <cstring> using namespace std; void AddStudent(string name[], int id[], int size); . . . void AddStudent(string name[10000], int size, int id[10000]) { bool valid; int num; do{ cout << "New Student's id: "; cin >> num; cout << "\n"; if((num > 0) && (num < size) && (num != id[num])) valid = true; else cout <<"Please enter valid entry for a new Student."; }while(valid = false); cout << "Enter new Student's name: " cin >> name[num]; cout << "\n" id[num] = num; }
Advertisement
Try..

#include <iostream>#include <cstring>using namespace std;void AddStudent(string *name, int *id, int size);...void AddStudent(string *name, int *id, int size){bool valid;int num;do{cout << "New Student's id: ";cin >> num;cout << "\n";if((num > 0) && (num < size) && (num != id[num]))valid = true;elsecout <<"Please enter valid entry for a new Student.";}while(valid = false);cout << "Enter new Student's name: "cin >> name[num];cout << "\n"id[num] = num;}


You should make the function prototype and the function definition the same in the terms of arguments (you swapped one around)
I believe the problem is in the caller function.

It looks like you are onlty passing 1 string, and not an array to the function.

You may also have problems with your valid variable.

1) you never set it to any value. (it would also be nice to initialise num to a value too, it's good style. 0 is good. )

2) the line while( valid = false ); looks like an assignment, and not an equality test, to me.

Quote:
You can't pass arrays as you were trying to do


Yes you can.
If I am understanding your code, you want the function to add one student. Your function just gets one student id. So why are you passing an array at all? Why not just return the id from the function?

Also your code has an error in it here:

cout <<"Please enter valid entry for a new Student.";
}while(valid = false);

In the while, you set valid to false which is false, so the loop never repeats.

I would rethink your design a little bit. (This looks like homework, so I will not be more specific).

PS When posting code, place it in <pre> </pre> tags to preserve formatting.
Thankyou for pointing out the switched variables, and the passing of a single value. I have finally gotten past that point.

BTW its for more than one student, so I have to use arrays.

Thanx fer da help!
0) Your prototype doesn't match the definition. That's a bad thing. When you say "void AddStudent(string[], int[], int);", that's saying you will later define it to take a string array, int array and int *in that order*. The definition you give has it in a different order. If you're not getting a compiler error for that, it's because you don't happen to need a prototype for the way you're using it (i.e. the calls to AddStudent all come after the definition), but you will later get a linker error complaining that the other version (which you promised) isn't present.

1) You should try to separate your I/O logic from your "business" logic. Get the values that you want first, and then do the update. In the provided code, I've moved the registry-update stuff into its own functions that don't do any I/O, though they still need to be called intermingled with the I/O operations. But when you can for example ask for all your information at once and then use it for a constructor call, do that instead.

2) The error you cited is in the line where you call the function, so you should be showing the code around the call as well.

3) The header you want is called string, not cstring. cstring defines some free functions designed for treating char*s as if they were strings. This is ugly stuff that you generally want to stay away from in C++.

4) When you write the [10000]s in the definition, they don't really mean anything. What will be passed is a pointer to the first element of the array, and your parameter is effectively of type pointer-to-whatever. Which is why the compiler error reads as it does.

5) Don't use arrays for this. Please. I assume what you want to do is modify the passed-in array by setting an element of it (according to the specified ID number). You have huge problems if the ID number is out of bounds. You also have memory management headaches as well as call-semantics problems (i.e. "how do I pass this thing?) with arrays, even if the input value is OK.

You can use a std::vector as more or less a drop-in replacement for an array. Because it's an object, you can pass it to a function (by reference, if you want to modify the contents; by const reference if you don't - by value works too, but is seriously inefficient, as it makes a useless temporary copy of the whole thing), return it from a function, and generally treat it as you would expect to be able to, rather than worrying about weird array vs. pointer semantics. (By the way, you should read this to educate yourself about pointers vs. arrays.)

However, in your case, you should probably be using something like std::map instead. This allows you to use just a few, very large ID numbers, without allocating space for entries in between. That is, only the student entries which you request will "exist", whereas with the vector (or an array) there are uninitialized student entries in all the other slots. It is also faster to insert things into the middle, or at the beginning of a map than into a vector. The tradeoff is that the map will require more memory per entry which actually exists, and be a bit slower for indexing, but you will probably not need to worry about either of those.

Another indication that the vector is a bad idea is that you have this id array that needs to be modified in sync with the names array to check the validity of the entries. (At the very least, you should wrap things in a struct: make a 'Student' struct containing a string name; and a bool valid;. You don't need an array with actual numeric values, because id[num] is only ever num or 0 currently; what you care about is the validity of a given Student entry, so you just flag that directly.)

You can also make use of a typedef to "protect" your code if you later change your mind about which container to use.

#include <map>#include <string>using namespace std;// The type of a "registry" of students.typedef map<int, string> registry_t;bool isStudentRegistered(const registry_t& registry, int id) {  return registry.find(id) != registry.end();}bool AddStudent(const registry_t& registry, int id, string name) {  // If already registered, can't add.  if (isStudentRegistered(registry, id)) { return false; }  // Otherwise, add the student and report success.  registry[id] = name;  return true;}// A useful helper function for reading input robustly.// Tries to read into one variable, and unconditionally "clears" to the next// line. Returns whether the read was successful.template<typename T>bool readIn(istream& is, T& data) {  bool result = (is >> data);  is.clear();  is.ignore(INT_MAX, '\n');  return result;}// Somewhere else...registry_t allStudents;// ...bool done = false;while (!done) {  int num;  string name;  cout << "New Student's id: " << flush;  if (readIn(cin, num)) {    // Note that this code doesn't detect an already-used ID until after a name    // is prompted for. If you wanted that behaviour, you can check explicitly    // for isStudentRegistered() here, and report an appropriate error.    // Got a number; clear line and ask for a name    cout << "Enter new Student's name: " << flush;    readIn(cin, name); // this will always succeed.    if (AddStudent(allStudents, num, name) {      done = true;    } else {      // Couldn't add the student.      cout << "There is already a student with that id, try again." << endl;    }  } else {    cout << "That is not a valid student ID - please type a number" << endl;  }}

This topic is closed to new replies.

Advertisement