Jump to content
  • Advertisement
Sign in to follow this  
RogerThat123

confusing pointers

This topic is 3236 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts


void Options::AddCar(Car *old_car, int &elements)
{
	Car * carTemp;
	carTemp = new Car[elements + 1];
	cout << "Created array of size: " << elements + 1 << endl;

	for (int i = 0; i <= elements; i++)
	{
		if (i == elements)
		{
			string sModel, sLicense;
			cout << "Enter the car model: ";
			cin >> sModel;
			cout << "Enter the car license tag: ";
			cin >> sModel;
			carTemp.model = sModel;
			carTemp.license_tag = sLicense;

			cout << i << "." << carTemp.model << endl;
		}
		else
		{
			carTemp.model = old_car.model;
			carTemp.license_tag = old_car.license_tag;
		}
	}

        delete [] old_car;
	old_car = carTemp;

	elements++;
}


I have a pointer to an array of my struct (Car). Car contains 2 fields, model and license_tag (both strings). Im trying to make a function to add an element dynamically of course. Elements is passed in as 1 because theres 1 car so far, so Im trying to increment the array to 2 and add in the information. As you can see I increase the size of it here
carTemp = new Car[elements + 1];

and in the loop Im copying from the old pointer array to the new one thats bigger, and at the end when i == elements (since arrays start at 0), I put the new information in. But right after i print the information out the MODEL, it doesnt output the model i just entered it outputs the license_tag which is really wierd. I probably made a mistake.

Share this post


Link to post
Share on other sites
Advertisement
You also need to pass the pointer in by reference also:


void Options::AddCar( Car*& old_car, int &elements) {
/* ... */
}



Is there any reason why you aren't using std::vector? You might also want to use std::getline, since the extraction operator for std::string is space (and newline) delimited.


void Options::AddCar( std::vector<Car>& cars ) {
Car temp;
std::cout << "Enter model: ";
std::getline( std::cin, temp.model );
std::cout << "Enter license: ";
std::getline( std::cin, temp.license_tag );

cars.push_back( temp );
}


Share this post


Link to post
Share on other sites
C++ uses pass by value for pointers. That is, the value of the pointer (an address) is copied into the argument. This means that changes to the value of the variable "old_car" (again, the address) will not be visible outside the function call.

Changes to the data pointed to by old_car are visible.

With this in mind, let us examine the logic of your function. It allocates a new array, one bigger than the old one. It loops through the old values, copying them across to the new array. It fills in data from the user for the new one. Finally, it deletes the contents of the incoming array, and assigns the new pointer over the argument pointer. It increments the number of elements.

The two bolded lines are the only changes the caller sees. Effectively, the function deallocates the array, increments the integer referenced by "elements", and leaks a bit of memory too.

A few minor things before I continue.

  • Your input loop reads the model twice, and never the license

  • Hungarian notation is pointless. Don't prefix strings with "s".

  • A simpler way to write your function is to write the loop from i = 0 and i being less than elements, and then writing the code to fill in the final element after the loop. This avoids a bunch of conditional statements, as well as clarifying the logic and flow of the function.

  • Why is this a member function? You don't appear to access any member variables or member functions, so this can be more cleanly implemented as a free function.



The simplest solution is to use std::vector:

#include <vector>
void AddCar(std::vector<Car> &cars)
{
string model, license;
cout << "Enter the car model: ";
cin >> model;
cout << "Enter the car license tag: ";
cin >> license;

cars.push_back(Car(model, license));
}


std::vector is a dynamic array which manges its own size and provides easy ways to add, remove and access the elements. It is your first port of call for simple "bag like" containers.

An example using raw pointers:

#include <algorithm>

void AddCar(Car *& original, int &elements)
{
Car *temp = new Car[elements + 1];

// copy the old elements across
std::copy(original, original + elements, temp);

string model, license;
cout << "Enter the car model: ";
cin >> model;
cout << "Enter the car license tag: ";
cin >> license;

temp[elements] = Car(model, license);

delete [] original;
original = temp;
++elements;
}


Another option might be to return the new pointer.

Share this post


Link to post
Share on other sites
std::vector is nice, but it's also valuable to learn what goes on behind vector, which is essentially what AddCar is trying to accomplish.

Share this post


Link to post
Share on other sites
Quote:
Original post by rip-off
C++ uses pass by value for pointers. That is, the value of the pointer (an address) is copied into the argument. This means that changes to the value of the variable "old_car" (again, the address) will not be visible outside the function call.

Changes to the data pointed to by old_car are visible.

With this in mind, let us examine the logic of your function. It allocates a new array, one bigger than the old one. It loops through the old values, copying them across to the new array. It fills in data from the user for the new one. Finally, it deletes the contents of the incoming array, and assigns the new pointer over the argument pointer. It increments the number of elements.

The two bolded lines are the only changes the caller sees. Effectively, the function deallocates the array, increments the integer referenced by "elements", and leaks a bit of memory too.

A few minor things before I continue.

  • Your input loop reads the model twice, and never the license

  • Hungarian notation is pointless. Don't prefix strings with "s".

  • A simpler way to write your function is to write the loop from i = 0 and i being less than elements, and then writing the code to fill in the final element after the loop. This avoids a bunch of conditional statements, as well as clarifying the logic and flow of the function.

  • Why is this a member function? You don't appear to access any member variables or member functions, so this can be more cleanly implemented as a free function.



The simplest solution is to use std::vector:
*** Source Snippet Removed ***
std::vector is a dynamic array which manges its own size and provides easy ways to add, remove and access the elements. It is your first port of call for simple "bag like" containers.

An example using raw pointers:
*** Source Snippet Removed ***
Another option might be to return the new pointer.


Thanks! I used your example, looks much tidier and more efficient. Thank you :)

Thanks to rest for your input as well

Share this post


Link to post
Share on other sites

string model, license;
cout << "Enter the car model: ";
getline (cin,model);
cout << "Enter the car license tag: ";
cin >> license;



This doesnt work, after I enter the model, it skips the next one, and skips every cin for the rest of my program sending it into and endless loop.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!