#Do not write using namespace std;
Do not write using namespace std;
#Do not write using namespace std;
Do not write using namespace std;
The include guard is too easily clashed with someone else's file when this is used. Use #pragma once
; or if a guard is really needed, use a UUID.
The coType
is best an enum, not a char.
Don’t write an empty destructor. Leave it off completely, or use =default
to emphasize that it exists. You’ve made your class to not be trivally destructable because of this, which means the compiler can’t do certain optimizations (that's an over simplification).
Learn the canonocal way to write arithmetic operators. That is, make +=
a member and then write +
in terms of it; etc. You don’t even have the parameter right: why can’t someone add a const value?
void foo (const Force& a, const Force& b)
{
Force c = a + b; // fails to compile!
Your operators are assuming that the arguments are in Cartesian representation. It will get the wrong results if in polar.
Will something like
Force baz (const Force& in)
{
Force res;
res = baz;
return res;
}
compile? Why not? Go over your checklist of functions you need in a class.
set_coType
is silly. It is meaningless to change that without changing the existing values. Now if you had a function to change the internal representation to the chosen form, that would be better.
Having set_r1
, set_r2
is not a normal thing for a simple numeric class like this. You would normally just form a new complex number, not change one of the parts of an existing value.
E.g. to get the complex conjugate of z
, write Force{'C', z.r1, -z.r2};
but now you see another issue; there is no way to ensure that I’m in C format to begin with nor a way to transform it into C mode.
If we had accessors like real
, imag
, abs
, arg
then those would return the desired value whether stored or computed. E.g. if I had a polar value, real
would calculate it from the polar information it has; if it was a Cartesian value, it just returns the first stored number.
I suggest modeling your complex numbers on the standard class, linked in the previous paragraph.
#Do not write using namespace std;
Your converter
function that switches from one to the other representation is overly complicated, and not very useful when you can’t tell which form you start with. E.g.
void bar (const Force& f1)
{
// I want that in polar mode.
Force f2= f1;
if (/* what?? no accessor defined to check for that */)
f2.convert();
⋮
Instead, have a way to convert to polar regardless of what I start with: if already in polar, it’s easy! Internally, have two separate functions with different variables; your x
, y
, ⋯ x
, rad1
is just too muddled to deal with.
Your comment "intermeditary variables used to stop math errors in the conversion" makes me think you just added them to fix a problem. Declaring all the variables you might need at the top of a function is bad. If you declared where needed, you see you only need half of those in each branch.
And the function to do the converting should be (drum roll...) the constructor!
Force f2 {f1,'P'};
an optional second argument to the copy constructor will convert if necessary.
print
should be a non-member function that is not a core part of the class. And what’s with the int return that’s always zero?
If you needed to make it a member to print coType
, you should instead take that as a sign that the public API is lacking the needed features to use the class effectively (e.g. return what is the current mode).
And why not allow printing of const
values?