homework help wanted - Class member functions

I'm in my 201 programming class and we have an assignment dealing with a class. I have a decent handle on everything so far, but right now I'm stuck with my code and I'm hung up on my class member functions, namely the accessor and mutator.

Code:

#include <iostream>
#include <math.h>
using namespace std;

class circle
{
  private:
    int radius;
    int circumference;
	int area;
  
  public:
    void caclulateArea();
	void calculateCircumference();
	void displayOutput();
	void setRadius();  //mutator
	int getRadius(int userRadius);  //accessor
};

int main () 
{
  int userRadius = 0;
  const double PI = 3.141592;
  
  circle test;
  circle::getRadius(userRadius);
  circle::setRadius();
  test.getRadius(userRadius);
  test.setRadius();
  test.caclulateArea();
  test.calculateCircumference();
}

/****************************** calculateArea 
Description  : Takes radius and squares it and multiplies it by pi
Parameters 
     input   : radius, PI
    output   : 
Returns      : 
Preconditions: must be a positive number
------------------------------------------------------------------------------*/
void calculateArea(int radius,const double PI)
{
  double area = pow (radius * PI, 2);
}

/************************** calculateCircumference 
Description  : Takes radius and squares it and multiplies it by pi
Parameters 
     input   : radius, PI
    output   : none
Returns      : none
Preconditions: none
------------------------------------------------------------------------------*/
void calculateCircumference(int radius, const double PI)
{
  double Circumference = (radius * PI * 2);
}

/****************************** displayOutput 
Description  : displays the answers on the console
Parameters 
     input   : none
    output   : radius, area, circumference
Returns      : none
Preconditions: none
------------------------------------------------------------------------------*/
void displayOutput(int radius, int area, int circumference)
{
  cout << "Using " << radius << " as the radius" << endl;
  cout << "The area would be " << area << endl;
  cout << "The circumference would be " << circumference << endl;
}

/****************************** getRadius 
Description  : gets the radius from the user 
Parameters 
     input   : userRadius
    output   : none
Returns      : userRadius
Preconditions: none
------------------------------------------------------------------------------*/
int circle::getRadius(int userRadius)
{
  cout << "This program will calculate the area and circumference of " << endl;
  cout << "a circle based off of the radius using a class." << endl;
  cout << "Enter the radius --> " << endl;
  cin >> radius;
  return userRadius;
}

/****************************** setRadius 
Description  : sets the radius into the class by way of userRadius
Parameters 
     input   : none
    output   : none
Returns      : none
Preconditions: none
------------------------------------------------------------------------------*/
void circle::setRadius()
{
  int radius = 0;
  int userRadius = radius;
}

I get these 2 errors when I compile...
1>c:\school\c201\c201 homework3\hw3.cpp(33) : error C2352: 'circle::getRadius' : illegal call of non-static member function
1> c:\school\c201\c201 homework3\hw3.cpp(24) : see declaration of 'circle::getRadius'
1>c:\school\c201\c201 homework3\hw3.cpp(34) : error C2352: 'circle::setRadius' : illegal call of non-static member function
1> c:\school\c201\c201 homework3\hw3.cpp(23) : see declaration of 'circle::setRadius'

I don't know if its something with my scope resolution or if I'm supposed to be using a dot operator instead. In the error messages it mentions static member functions but we haven't covered that and the professor said not to use any static functions or variables. Thanks in advance for any help.

One of the most obvious problems is that you are calling some functions like this:

Code:

 circle::getRadius(userRadius);
 circle::setRadius();

This is the syntax for static member functions, and if you look at the class, neither of these functions is a static one. Can you make it a static function? Sure. But that probably doesn't make sense if you fully understood what static means in this context. Static functions can be called without an object, which means that they can be called when there are no circles. It doesn't make sense to access or mutate parts of a nonexistent circle.

So you call them with an object in the usual way:

Code:

circle test;
test.setRadius();
test.getRadius(userRadius);

You might be asking how to give circle values when you want to create a circle, so that these functions, when they are called, make sense. Giving an object its first values (initial values) is done by a constructor.

The next obvious problem is that none of your functions are set up to take arguments like userRadius. You might have void setRadius(int userRadius); in a class that you can call like test.setRadius(4); or whatever. The function changes the thing that represents radius inside the class, and you are basically done. EDIT: Hmm, it appears you flip-flopped the usual meaning of get and set, which led to my confusion. What you have is actually fine, apart from static. "get" is usually an accessor and "set" is the mutator though.

Boy, where to begin...

#1

Code:

class circle
{
...

    void caclulateArea();
    void calculateCircumference();
    void displayOutput();
    void setRadius();  //mutator
    int getRadius(int userRadius);  //accessor
};

...

void calculateArea(int radius,const double PI)
{
    ...
}

...

void calculateCircumference(int radius, const double PI)
{
    ...
}

...

void displayOutput(int radius, int area, int circumference)
{
    ...
}

...

int circle::getRadius(int userRadius)
{
    ...
}

...

void circle::setRadius()
{
    ...
}

You manage to set up the setRadius and getRadius functions correctly by indicating they are a part of the circle class but you neglect to do so with the calculateArea, calculateCircumference and displayOutput functions? Those both need circle:: in front of the function names.

#2

Code:

class circle
{
private:
    int radius;
    int circumference;
    int area;
...
};

...

void calculateArea(int radius,const double PI)
{
  double area = pow (radius * PI, 2);
}

...

void calculateCircumference(int radius, const double PI)
{
  double Circumference = (radius * PI * 2);
}

...

void displayOutput(int radius, int area, int circumference)
{
  cout << "Using " << radius << " as the radius" << endl;
  cout << "The area would be " << area << endl;
  cout << "The circumference would be " << circumference << endl;
}

...

int circle::getRadius(int userRadius)
{
  cout << "This program will calculate the area and circumference of " << endl;
  cout << "a circle based off of the radius using a class." << endl;
  cout << "Enter the radius --> " << endl;
  cin >> radius;
  return userRadius;
}

...

void circle::setRadius()
{
  int radius = 0;
  int userRadius = radius;
}

Your class has member variables. Use them! Most of your member functions take arguments that are unnecessary - you should not have to pass in a radius, area, circumference values to these functions since they should just be able to access the class's own variables. The constant PI should be defined as such somewhere at the beginning of the program as a global and not in main, it therefore also does not need to be passed into the functions.

You use local variables in your member functions with the same/different names as the class's own member variables but the local ones will hide the class's ones and changes to the locals will therefore not affect the class variables. The only place you appear to do this correctly (somewhat) is in your getRadius function where you reference the member variable radius (why are you returning the same value you pass in). Everywhere else, you have locals that simply get a value assigned and then are immediately thrown away as they are popped off the stack when the functions end - this is pointless.

#3 Design issues - Your current class design depends on the calculateArea, calculateCircumference functions being manually called after each update to the radius. It would be better for these functions to be called automatically within whatever setRadius function you wind up with such that when the radius gets updated, the new circumference and area values are updated along with it instead of relying on you manually calling the relevant functions.

If your getRadius function is an accessor as you claim then it should simply return the current radius value stored in the class. I/O does not seem appropriate in such a function. It does not need an argument passed into the function, simply return radius (and don't declare a new local radius variable). As such this function should probably be declared const since it would not then be a function that modifies the class.

The setRadius function should either accept a value which holds the new desired radius - to which the classes radius variable gets set - or it should query the user and accept a new value through cout/cin.

...probably other stuff but that's it for now.

So in other words my setRadius and getRadius functions are backwards? It makes sense in the sense that my program runs but when I try to pass my userRadius value into set it gets lost after the first function call. Here is my updated, and working, code. But when I debug I see that nothing gets set into radius and everything comes out with 0's at the end.

Code:

#include <iostream>
#include <math.h>
using namespace std;

class circle
{
  private:
    int radius;
    double circumference;
    double area;
  
  public:
    void calculateArea();
    void calculateCircumference();
    void displayOutput();
    void setRadius(int userRadius);  //mutator
    int getRadius();  //accessor
};

const double PI = 3.141592;

int main () 
{
  circle test;
  int userRadius = 0;

  test.getRadius();
  test.setRadius(userRadius);
  test.calculateArea();
  test.calculateCircumference();
  test.displayOutput();
}

void circle::calculateArea()
{
  area = pow (radius * PI, 2);
}

void circle::calculateCircumference()
{
  circumference = (radius * PI * 2);
}

void circle::displayOutput()
{
  cout << "\nUsing " << radius << " as the radius" << endl;
  cout << "The area would be " << area << endl;
  cout << "The circumference would be " << circumference << endl;
}

int circle::getRadius()
{
  int userRadius = 0;

  cout << "This program will calculate the area and circumference of " << endl;
  cout << "a circle based off of the radius using a class." << endl;
  cout << "Enter the radius --> ";
  cin >> userRadius;
  return userRadius;
}

void circle::setRadius(int userRadius)
{
  radius = userRadius;
}

I guess my main flaw is not understanding the accessor and mutator functions. My lecture notes all have the get and set functions as I do. Does the GET function get the data from the class or from the user? And as far as running everything from within my setRadius function wouldn't that contradict using the seperate class functions?

Ok, it seems the purpose of your getRadius function is to actually obtain/get the desired radius value from the user. In this regards, it is different from what most would think of which is to return the existing class's private radius member as is typical with most "get" functions. Personally I'd remove the I/O from the function and have it placed outside the class. I'd ask the user for a desired radius value in main and I would pass that value to the setRadius function.

If you truly want your getRadius function to be responsible for the I/O, then there is no reason to have it return a value and there would also be no reason to have a setRadius function. You could just read the user input straight into the class radius variable and be done with it.

My main function would look more like:

Code:

int main () 
{
    circle test;
    int userRadius = 0;

    cout << "Enter the desired radius: ";
    cin >> userRadius;

    test.setRadius(userRadius);
    ...

...which does not then require a getRadius function.

[edit]#include <math.h> should be #include <cmath>[/edit]

The thing that I never 100% understood was why the SET and GET functions have to be seperate. I don't understand why you can't just have one function to do both. We're supposed to use corresponding mutators and accessors with our functions and thats why I tried to incorporate both. I guess the GET function doesn't have to ask the user for the data, but what would its general purpose be without it?

So called accessor functions are one way, and I say the easiest way, for the class to share its member data with the program, outside of the class. For example, the standard string class has size(), which could be thought of as returning a length variable from inside the class. You can see hopefully why information like that would be necessary to share. Even though that is not the only way to use classes, because it is so simple to do people learn that first.

set/get functions promote encapsulation, an important concept to protect your code from changes. You can read up about it on Wikipedia (or other sites).

I read through my text and various online sources and understand encapsulation. I guess my fault is in how to construct the mutator and accessor functions. I know I can just take the userRadius from main and plug it in, but then I'm missing part of the application. My text has information on it, and as I have read and you guys have said - it makes sense but I can't seem to formulate them without either combining them or making one useless and unnecessary.

I can't seem to formulate them without either combining them or making one useless and unnecessary.

That's OK.

Just as an example:

Code:

double circle::getArea() const // normal method
{
    return M_PI * radius * radius;
}
double circle::getRadius() const // accessor
{
   return radius;
}
void circle::setRadius(double newRadius) // mutator
{
   radius = newRadius;
}

How many ways can you write the same thing?

Accessors and mutators are just code around what could be a public variable. I'm firm in the opinion that you can write them without "formulating them." All the accessor and mutator code is very similar looking whenever you write it. Whether it is necessary or not is up to the individual, as long as what needs to be done is in fact done.

Thanks for all of the help, I really appreciate it. Here is a copy of my "final" and hopefully properly working code. I get no errors and no warnings and my answers come out correct every time I run it. If anything looks funny or incorrect let me know.

Code:

#include <iostream>
#include <cmath>
using namespace std;

class circle
{
  private:
    double radius;
    double circumference;
	double area;
  
  public:
    double calculateArea();
	double calculateCircumference();
	void displayOutput();
	void setRadius(double userRadius);  //mutator
	double getRadius();  //accessor
};

const double PI = 3.141592;

int main () 
{
  circle test;
  double userRadius = 0;

  cout << "This program will calculate the area and circumference of " << endl;
  cout << "a circle based off of the radius using a class." << endl;
  cout << "Enter the radius --> ";
  cin >> userRadius;

  test.getRadius();
  test.setRadius(userRadius);
  test.calculateArea();
  test.calculateCircumference();
  test.displayOutput();
}

double circle::calculateArea()
{
  area = pow (radius, 2) * PI;
  return area;
}

double circle::calculateCircumference()
{
	circumference = (radius * PI * 2);
	return circumference;
}

void circle::displayOutput()
{
	cout << "\nUsing " << radius << " as the radius" << endl;
	cout << "The area would be " << area << endl;
	cout << "The circumference would be " << circumference << endl;
}

double circle::getRadius()
{
  return radius;
}

void circle::setRadius(double userRadius)
{
  radius = userRadius;
}

My last question is am I using the accessor and mutator correctly this time around? Or did I simply end up taking a shortcut by taking the input by cin through main?

/sigh The more I look at it the more the accessor doesn't make sense to me. By taking the userRadius in main it makes my getRadius function unnecessary. How can I properly incorporate my getRadius function?

There are several things you must understand:
- We are modelling the circle class after a real circle.
- Does it make sense for the circle to ask for its radius? It does not. Furthermore, it hurts the genericity of your code. That is, you limit the number of ways the class can be used. We want to control how to how and when to set the radius. Thus, there shall be no cin inside the class.
- Get should absolutely and under no circuimstances ASK for a radius! Get is as its name implies - GETTING the existing radius. Thus, if you have to ask for radius in there, then your design is horribly broken. And what is the point of asking the user for a radius and returning it only? Then that function really has nothing to do at all with the class!
- Why would we want a GetRadius? Consider some code that works with the circle, for example, drawing it. It would then need to know the radius, right? So either you keep around a variable that keeps track of the radius, or you let the class--object--itself keep track of the radius. The later is preferred of course. And what if you have several circles? And what if some are user defined and some are hard-coded? If you ask the circle for its radius, you know that it's always correct.
- The reason GetRadius returns 0 is because you're calling it before SetRadius.

Everything you said makes perfect sense now. I think I'm finally starting to understand. My current program is pretty much everything I listed last time, except that I did switch around my get and set functions already. But why have my get function when I can take it out of main and everything still works?

Just because the function is there does not mean you have to use it. In this elementary program, you don't need to use the get function.
It's the same as asking: why does a car have a radio if I can take it out and it still works? Because you might want it someday.

Point well taken. Thanks for all the help, its greatly appreciated. My program works correctly, and seemingly to myself effeciently, so I'm calling it finished. Thanks again.