C++ problems

neilk32

I have an exam coming up and I'm a complete beginner at c++, wondering if somebody could help me out with these two problems. Explanations would be very helpful.

a)
Explain why the following C++ function does not behave correctly and correct it:

void swap(int a, int b)
{
int temp;
temp = a;
b = a;
a = temp;
}


b) Explain why the following C++ code, which returns a count of the number of live entities in a vector is unnecessarily inefficient and improve it.

std::vector<Entity>::size_type liveCount(std::vector<Entity> v)
{
std::vector<Entity>::size_type count = 0;
for( std::vector<Entity>::size_type i = 0; i != v.size(); i++ )
if( isLive (v ) ) ++count;
return count;
}


Thanks for your help.

C1ancy

The first one doesnt behave correctly because functions create copies of variables given to it and operate on them, not where the original value is stored. If they had used reference variables (int&) it would have been correct I think.

Not sure about the second q.

Groinshot

C1ancy wrote: »

The first one doesnt behave correctly because functions create copies of variables given to it and operate on them, not where the original value is stored. If they had used reference variables (int&) it would have been correct I think.

Not sure about the second q.

Doesn't work because a is never changed. You set the value of temp to the value of a, then set b = to a, and then let a = temp. you end up with a and b equal to each other.

fixing it depends on how you want to do it.
I'd personally go for
void swap(int a, int b){
a= a + b;// lets a = sum of the two numbers.
b = a - b;// swaps the value from a into b
a = a - b; // puts b's old value into a
}

second function:?

fasty

Groinshot, what the hell code is that supposed to do?

Groinshot

fasty wrote: »

Groinshot, what the hell code is that supposed to do?

swaps the two numbers. Think about it.

if you let a = 2, b = 3.

a = a+b; //a = 5, b = 3
b = a - b;// a = 5, b = 2
a = a - b;// a = 3, b = 2

smallapple

In c parameters can be passed either by reference or by value.

In your swap function the parameters are passed by value, which means the value of a and b are passed to the function. If you want to modify a or b then you must reference the variable directly by passing the references to variables.

Change your function to use references

void swap (int& a, int& b)

Hope this helps, ignore groinshots answer!

CrinkElite

a)
Explain why the following C++ function does not behave correctly and correct it:

void swap(int a, int b)
{
int temp;
temp = a;
b = a;
a = temp;
}

I'm no coder but I think groinshot must be at least half right.
the B value is erased when it is given the value of A and therefor lost.
you end up with A and B both containing the original value of A.

Correct me if I'm wrong.

It should be

void swap(int a, int b)
{
int temp;
temp = b;
b = a;
a = temp;
}

don't know much about reference/value craic.

Groinshot

smallapple wrote: »

In c parameters can be passed either by reference or by value.

In your swap function the parameters are passed by value, which means the value of a and b are passed to the function. If you want to modify a or b then you must reference the variable directly by passing the references to variables.

Change your function to use references

void swap (int& a, int& b)

Hope this helps, ignore groinshots answer!

You do need to use a reference, but the code is still wrong.

//assume a is 2, b is 3.
int temp;
temp = a; // temp is 2, a is 2, b is 3
b = a; // temp is 2, a is 2, b is 2
a = temp; // temp is 2, a is 2, b is 2

bolded the changing variable each time.

neilk32

void swap(int a, int b)
{

int temp;

temp=a;

a=b

b= temp

}

A classmate said this is right but it doesn't look much different to me thoughts?

smallapple

What are you on about, a void function does not return a value, it's got nothing to do with parameters. Should you really be answering programming questions?

joe_chicken

Use references. Or pointers.

void swap(int *a, int *b)
{
int temp;

temp = *a;
*a = *b;
*b = temp;
}

Because of the way c/c++ works, the arguments ( a and b) are copied to local scope variables inside the swap function, so making any changes on them will not be reflected outside the function. You can get around it by passing in a reference or pointer to the variable and then change the contents of that pointer or reference.

The second function, I presume they're looking for some kind of iterator answer, not sure.

tehjimmeh

Groinshot wrote: »

Don't need to reference in a void function.

Wtf?

Groinshot

neilk32 wrote: »

void swap(int a, int b)
{

int temp;

temp=a;

a=b

b= temp

}

A classmate said this is right but it doesn't look much different to me thoughts?

That seems right. But as someone rightly pointed out my mistake, you do need to use a reference. And smallapple, I know it doesn't return a value, I never said it did, I was mistaken, (home now) But that doesn't change the fact that the algorithm was still wrong and I pointed that out.

Mutant

Q1
void swap(int &a, int &b)
{
int tmp;
tmp=a;
a=b;
b=tmp;
}

Q2


std::vector<Entity>::size_type liveCount(const std::vector<Entity> &v) {
std::vector<Entity>::size_type count = 0;
for( std::vector<Entity>::size_type i = 0; i != v.size(); i++ ){
if( isLive(v ) ){
++count;
}
return count;
}
}

You could also use iterators instead of indexing

Axe Rake

neilk32 wrote: »

Explain why the following C++ function does not behave correctly and correct it:

void swap(int a, int b)
{
    int temp;
    temp = a;
    b = a;
    a = temp;
}

As the others have explained, the value of b is lost in the above function and both variables end up with the same value.

Correct piece of code is:

void swap(int a, int b)
{
    int temp;
    temp = a;
    a = b;
    b = temp;
}

Or if you want to be really fancy and swap the variables without a temporary and include pass by reference:

void swap(int& a, int& b)
{
    a = a + b;
    b = a - b;
    a = a - b;
}

srsly78

Your "really fancy" way fails in the general case because of overflow. Expect to be marked down.

Axe Rake

srsly78 wrote: »

Your "really fancy" way fails in the general case because of overflow. Expect to be marked down.

There was no specification given for the range of possible variables that can be passed in so we can only assume. But yes you would be correct if the passed in variables are large enough.

Anima

we can only assume

You should assume the worst then no?

Anyway, a single stack variable isn't going to kill anyone. In fact I wonder if all the subtractions etc is more expensive than just a few assignments, probably.

Giblet

Yeah otherwise people would be like

if (a != b){
    *a ^= *b ^= *a ^= *b
}

without a care in the world.

tehjimmeh

Anima wrote: »

You should assume the worst then no?

Anyway, a single stack variable isn't going to kill anyone. In fact I wonder if all the subtractions etc is more expensive than just a few assignments, probably.

It probably is. It's one of those "clever" things that's quite silly to do in practice for a whole number of reasons.

Naikon

The XOR swap may be even less efficient nowadays then you think. We have modern instruction pipelines to thank for that.

Giblet

Ah it's pretty bad like!

Who has ever swapped two primitive variables anyway...

srsly78

Swapping variables is serious business :pac:

Webmonkey

srsly78 wrote: »

Swapping variables is serious business :pac:

srsly? :pac:

ShowMeTheCash

OK Ignore any answers referring to ref or pointers this is not what the question is asking.

The problem is very simple and as already pointed out easily answered.

void swap(int a, int b)
{
int temp;
temp = a;
a = b;
b = temp;
}


The second question is very inefficient for a number of reasons.

But the big one is v.size(), when you call v.size() it does not just return the size of the vector it actually iterates though the vector to retrieve the size.

So lets say the vector has a million elements, the v.size() will actually run through each element and return its own count.

And you are calling it in the for loop so each time the for loop iterates v.size() gets called so the vector gets iterated million x million times.

Using the same structure the correct code should look like

std::vector<Entity>::size_type liveCount(const std::vector<Entity> &v) {
std::vector<Entity>::size_type count = 0;
int vectorSize = v.size(); // CALLED ONCE

for( std::vector<Entity>::size_type i = 0; i != vectorSize ; i++ ){
if( isLive(v ) ){
++count;
}
return count;
}
}


This way v.size() only gets called once.

Easy!

Naikon

Real men manually construct and use their own linked list library:D

tehjimmeh

ShowMeTheCash wrote: »

OK Ignore any answers referring to ref or pointers this is not what the question is asking.

The problem is very simple and as already pointed out easily answered.

void swap(int a, int b)
{
int temp;
temp = a;
a = b;
b = temp;
}

Of course you need refs or pointers. That function does nothing.

srsly78

Correct, he would get a big fat zero mark for that.