• Advertisement
Sign in to follow this  

confusing pointers

This topic is 2993 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
Is it possible so when I enter model "Ford Fusion" it will put that into one string and not automatically put Fusion into the license_tag.

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
std::getline() works. What tends to cause problems is mixing std::getline() and the regular stream extraction operator, if you don't understand how they work. std::getline() consumes the newline in the buffer, where as the stream extraction operator will leave it present. Subsequent getline() calls will return an "empty" line. There is another issue regarding this, but I have totally blanked it at the moment. It has been a while since I've wanted to do console input with C++, forgive me. Maybe searching the forums might give you the information, in particular look for posts by "Zahlman".

This sample program might demonstrate better:

#include <string>
#include <sstream>
#include <iostream>

std::string handle(char c)
{
switch(c)
{
case ' ': return "<space>";
case '\t': return "<tab>";
case '\n': return "<newline>";
default: return std::string(c, 1);
}
}

void peek(std::istream &stream)
{
int value = stream.peek();
if(value < 0)
{
std::cout << "peek: end of stream\n";
}
else
{
char c = static_cast<char>(value);
std::cout << "peek: \'" << handle(c) << "\'\n";
}
}

int main()
{
const std::string source = "Hello world.\n How\tare\t\t \nyou?\n\nWhat's up?";

std::string element;

std::stringstream individual(source);

while(individual >> element)
{
std::cout << "individual: \"" << element << "\"\n";
peek(individual);
}

std::stringstream lines(source);

while(std::getline(lines, element))
{
std::cout << "lines: \"" << element << "\"\n";
peek(lines);
}
}



What I do for C++ console applications is to use getline() exlusively with std::cin, and then "reparse" the line using a std::stringstream instance if necessary. This has the added advantage of simplying error handling, because you don't need to remove the "failed" state from std::cin, the only stream that gets failed is the temporary stringstream.

Share this post


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

  • Advertisement