pass class to function fail

[like2codecpp]

ok so i have a function that i want to use an object as input for but when i do nothing happens the function doesn't execute but no errors appear. so anyway here is my code:

class px
{
	public:

	int x;
        
        int w;

	px(int a)
	{
		x = a;
	}

	void move(int nx)
	{
		x = x + nx;

		}
	}
};

void step(px part)
{
	
	speed = part.w * 7;

	part.move(0, speed, 0);

}

int main()
{

	px obj(1);

	step(obj);

	cout << "x = " << obj.x << "\n";
	
        system("pause");

	return 0;

}

for some reason obj.x stays at 1 and doesn't get updated by the step(); function.
however when i make obj a global variable and change my step(); function to :

void step()
{
	
	speed = part.w * 7;

	obj.move(0, speed, 0);

}

it works but obj needs to stay a static variable how can i fix my original code to make it work?

[sadavied]

First, study classes and functions a little more. For one thing, you declare your constructor to take 4 int parameters but instantiate the object with only 1?!? Then I see where you declare a member function 'move' that takes one int parameter, but in the step function, you pass 3?!? I'm not sure how that can even compile, let alone run!

Explain to me in plain English what this program is intended to do, without using code. Then I should be able to guide you.

[like2codecpp]

dude sorry that was i typo i was just simplifying my code to try to isolate the problem and i forgot to change that just pretend it says

px(int a)
	{
		x = a;
	}

its edited now
basically its supposed to add the value of speed to to obj.x the reason i need it to be this complex is that it will then use the variables that would be x y z to place it on a three dimensional grid but all that is not shown here. basically its a simple physics simulator. w is the weight of the object and 7 is the force of gravity.

[sadavied]

Ok, first let's take a look at your class. Normally, the raw data members of the class should be declared private or protected. So, in your case the two int members x and w should be declared private:

// px.h
#ifndef PX_H
#define PX_H

class px
{
public:
private:
  int x, w;
};
#endif 

Now, building from there, you will declare your public members. These will include your constructor and any member functions that you want to expose to users of your class. Adding that in would make it now look like this:

// px.h
#ifndef PX_H
#define PX_H

class px
{
public:
  px(int);
  px& move(int);

private:
  int x, w;
};
#endif 

Now, this presents a problem with your original requirements. You need access to the private member 'w'. So, the correct way to handle this is to create 'accessor' methods. These are simply set and get methods. Now, your class will look like this (I have also added a virtual destructor just in case you might want to inherit from this class later):

// px.h
#ifndef PX_H
#define PX_H

class px
{
public:
  px(int);
  virtual ~px();

  px& move(int);
  int getW() const;
  px& setW(int);

private:
  int x, w;
};
#endif 

Instead of having to create an instance of 'px' and then calling the 'set' method for variable 'w' to get a value into it, you should add that as one of the parameters of your constructor:

// px.h
#ifndef PX_H
#define PX_H

class px
{
public:
  px(int, int);
  virtual ~px();

  px& move(int);
  int getW() const;
  px& setW(int);

private:
  int x, w;
};
#endif 

^^This will be what the completed class definition looks like and this is in a file named 'px.h'.



Ok, you might be asking what some of that stuff is in the code I have provided. The whole point of creating classes and such is for usefulness in a program. One of the conventions that should be followed for EVERY class is splitting the definition from the implementation. You define the class in a header file and implement the class in a cpp source file. To this point, we have only dealt with the header file.

The way you will 'link' everything together is like this:

px.cpp:

#include "px.h"

// implementation code here

main.cpp:

// any includes that you need
#include "px.h"

int main(int argc, const char* argv[])
{
  // program code

  return 0;
}

I'll help get you started with the implementation file:

// px.cpp
#include "px.h"

px::px(int x_param, int w_param)
{
}
px::~px()
{
  // don't need to do anything here, just for inheritance purposes
}

px& px::move(int move_val)
{
}

int px::getW() const
{
}

px& px::setW(int w_val)
{
}

Go ahead and take a shot at completing the implementation file given the class I have helped you get started on. If you have specific questions about how to do something or what something here means, then ask. Please be specific.

[ne555]

If you want to modify the arguments of your function, pass them by reference.

void step(px part); //pass by copy
void step(px &part); //pass by reference 

@sadavied: if you are going to make setter/getters that "do nothing", just make your member public.

[sadavied]

One of the techniques of the object-oriented paradigm is encapsulation. Allowing users of your class to have public access to the data portion of your class violates this rule. This is why I use accessor methods for some data members. I provide, in the public interface, a way to obtain or modify the value of private data members without violating encapsulation.

I didn't mean for the OP to leave those member functions blank in my post above. I left it up to him/her to implement the function on his/her own. Now, I totally agree that those methods could be completely eliminated since both private members are initialized in the constructor. But, I was hoping the OP would get an idea of how to create a class by including them as a perfectly valid technique.

[ne555]

Why setter and getter methods are evil http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html?page=1

Don't ask for the information you need to do the work; ask the object that has the information to do the work for you.

[like2codecpp]

I get errors saying :

error C4716: 'px::move' : must return a value
error C4716: 'px::setW' : must return a value

I don't see how i can return a value of px&.

whats wrong?

[sadavied]

In those functions you are modifying the private data, so you are changing the object. Return a pointer to 'this':

px& px::setW(int w_val)
{
  w = w_val;

  return *this;
}

[ne555]

Or change it to void px::move(int)
Why returning something if you aren't going to use it.

[sadavied]

Because then you could do something like this:

px p(1);
p.SetW(7).move(3);

Without returning the reference, you would not be able to do that.

[sadavied]

ne555...

Thank you for posting that article. It was a very interesting read. Even though it is written with Java in mind, I think there are some valid points made there that apply to OOP in general. It has made me really start thinking about my own coding habits and what I should consider changing. I have thus started combing the internet for whatever I can find on coding standards/conventions etc.

Scott