Review My Code.

[bb0yth0r]

Hello everyone! I would just like to ask if my code is acceptable, and see if anyone can give me tips to be a better writer.

The Instructions Are:
Inside the main.cpp file demonstrate the Sphere class by creating a Sphere object called firstSphere such that the object is created by the default constructor. Verify the default constructor worked by calling the "Get" function and outputting the sphere’s radius.

Next create another sphere object called secondSphere and set the radius to -20.0 using the working constructor. Verify the working constructor worked by calling the "Get" function and outputting the sphere’s radius.

Finally, use the "Set" function on the secondSphere object to set the radius to 2.5. Next, call the SphereArea function to get the sphere’s area and call SphereDiameter to get the sphere’s diameter and output both values to the console.

My Main.cpp

#include <iostream>
#include "Sphere.h"
using namespace std;
int main()
{
	Sphere firstSphere;
	cout << "The Radius Of The First Sphere Is " << firstSphere.getRadius() << endl;
	Sphere secondSphere(-20);
	cout << "The Radius Of The Second Sphere Is " << secondSphere.getRadius() << endl;
	secondSphere.setRadius(2.5);
	cout << "The Area Of The Second Sphere Is " << secondSphere.SphereArea(2.5) << " With A Diameter Of " << secondSphere.SphereDiameter(2.5) << endl;
	return 0;
}

My Sphere.cpp

#include <iostream>
#include "Sphere.h"
Sphere::Sphere()
{
	this->myRadius = 1.0;
}
Sphere::Sphere(double myRad)
{
	this->myRadius = myRad;
}
double Sphere::setRadius(double Rad)
{
	myRadius = Rad;
	if (Rad < 1.0)
	{
		Rad = 1.0;
	}
	return Rad;
}
double Sphere::getRadius()
{
	return this->myRadius;
}
double Sphere::SphereArea(double Rad)
{
	myRadius = Rad;
	return 4.0 * PI * Rad * Rad;
}
double Sphere::SphereDiameter(double Rad)
{
	myRadius = Rad;
	return Rad + Rad;
}

My Sphere.h

#ifndef SPHERE
#define SPHERE
class Sphere
{
private:
	double myRadius;
	const double PI = 3.14159;
public:
	Sphere();
	Sphere(double myRad);
	double setRadius(double Rad);
	double getRadius();
	double SphereArea(double Rad);
	double SphereDiameter(double Rad);
};
#endif 

[48bpfSEw]

there are a lot of conventions which your code does not fullfill. beginning with the lack of comments, formating, conventions in naming of variables etc.

And where are the unit-tests?

https://google.github.io/styleguide/cppguide.html

[Chervil]

I would recommend using const where possible, function getRadius() does not change the object, so declare it const. There's no need to use the this pointer in the code at all.

double Sphere::getRadius() const
{
    return myRadius;
}

The area and diameter functions should not have an input parameter, instead they should use the value of the sphere's own radius. Make these const too.

PI should be a static variable, which means there is just one value for the entire class, rather than each sphere having its own separate value of PI.

Favour initializer list syntax in the constructors, for example instead of

Sphere::Sphere()
{
	this->myRadius = 1.0;
}

do this:

Sphere::Sphere() : myRadius(1.0)
{ }

[ne555]

	double SphereArea(double Rad);
	double SphereDiameter(double Rad);

¿why do these functions ask for a paramater?

Your constructor is not consistent with your setter. For example, you may construct the sphere with a negative radious, but the setter would not allow that.

[Aaron Vienneau]

Some people may also tell you that polluting the namespace with using directives is a bad idea. using namespace std; for example. This makes all names in the std namespace visible. If multiple namespaces are introduced with using directives, it can cause something called namespace clash.

As an alternative, you may consider qualifying each call to an std object with std:: . I've personally found this helpful because it helps me have a deeper understanding from where I'm getting these seemingly magic objects.

#include <iostream>
#include "Sphere.h"

int main()
{
	Sphere firstSphere;
	std::cout << "The Radius Of The First Sphere Is " << firstSphere.getRadius() << std::endl;
	Sphere secondSphere(-20);
	std::cout << "The Radius Of The Second Sphere Is " << secondSphere.getRadius() << std::endl;
	secondSphere.setRadius(2.5);
	std::cout << "The Area Of The Second Sphere Is " << secondSphere.SphereArea(2.5) << " With A Diameter Of " << secondSphere.SphereDiameter(2.5) << std::endl;
	return 0;
}

With such a small program, it's definitely a nit-pick but hopefully you aspire to one day make larger programs where this advice could come in handy.

[bb0yth0r]

@Necip What are unit tests? Mr professor does not show us how to do unit test he only taught us how to debug and start without debugging.

@Chervil Thanks for the tips! I forgot I had set the radius in the function before so I made it with an input parameter.

@ne555 Thanks for pointing that out! Oh yeah aha i forgot to mention that my professor wanted the setter to be 1.0 and if the radius was less than 1.0, set it to 1.0.

@Aaron Vienneau Aha thanks. I was told about that a bit ago, but my professor seems to always almost use using namespace std; in all of the examples he shows us. So I used it like that too. Will keep in mind, and yeah hopefully I can write better code in the future.

Thanks for the tips everyone!
...../ )
.....' /
---' (_____
......... ((__)
..... _ ((___)
....... -'((__)
--.___((_)

[48bpfSEw]

Unit-Test:
https://en.wikipedia.org/wiki/Unit_testing

These are function which prooves the functionality of your code!


Development is like climbing on a mountain.
You have to secure mostly every step (prooven by unit-tests or function-tests),
then you can be shure that your solution is stable.

[bb0yth0r]

@Necip Thanks aha i did not know that. will keep in mind :D