what'ch think of my fraction program, is it correct and good and efficient?

[John Cho]

// CHO, JOHN

#include<iostre am>


class fracpri{
int whole;
int numer;
int denom;

public:
// constructors:

fracpri();
fracpri(int w, int n, int d);
fracpri(float f); // float to class constructor

// member functions to show output and get input
void getFraction();
void showFraction();

// function and operators to do artimathetic
fracpri addfracts(fracp ri &obj1, fracpri &obj2);
fracpri operator+(fracp ri &obj2);
fracpri operator-(fracpri &obj2);
fracpri operator*(fracp ri &obj2);
fracpri operator/(fracpri &obj2);

// adding with constants
friend fracpri operator +(int i, fracpri &obj2);
friend fracpri operator +(fracpri &obj1, int i);

// the compare operator, <
bool operator<(fracp ri &obj2);

// the += operator
fracpri& operator+=(frac pri &obj2);

// overloading the input and output, >> and << operators.
friend std::istream& operator>> (std::istream& in, fracpri& obj2);
// std:: is used because
friend std::ostream& operator<< (std::ostream& out, fracpri const&
obj2);
// namespace std is after it


void reduce();

operator float(); // class to float conversion

};

using namespace std; // namespace needs to be after any use of
friend
// or else msvc++ 6.0 gives errors


fracpri::fracpr i(){
whole = 0;
numer = 0;
denom = 1; // it is set as 1 so the adding fractions
works with a
// common denominator and not 0
}

fracpri::fracpr i(int w, int n, int d){
whole = w;
numer = n;
denom = d;
if(denom == 0)
denom = 1;
reduce();
}

fracpri::fracpr i(float f){ //float type to class type constructor

f += 0.0078125; // .0078125 so numerator is rounded up later
whole = int(f);
float flofrac = f - whole; // the fraction part of the number
numer = (flofrac * 64); // .0078125 * 64 = .5 ; so rounded up
numerator
// if orignal fraction
denom = 64;
reduce();
}

void fracpri::getFra ction(){
char ch;
cout << "Enter a stock price in the form of " << endl;
cout << "wholenumbe r-numerator-denominator" << endl;
cin >> whole >> ch >> numer >> ch >> denom;
if(denom == 0)
denom = 1; // fix the denom if it is 0
reduce();
}

void fracpri::showFr action(){

cout << "The stock price is: " << endl;
cout << whole << ' ' << numer << '/' << denom << endl;

}





fracpri fracpri::addfra cts(fracpri &obj1, fracpri &obj2){
whole = obj1.whole + obj2.whole;
numer = (obj1.numer * obj2.denom) + (obj2.numer * obj1.denom);
denom = obj1.denom * obj2.denom;

reduce();
return *this;
}

fracpri fracpri::operat or +(fracpri &obj2){
fracpri temp; // temp is here because *this would modify
// the first object or a in
// c = a + b
temp.whole = whole + obj2.whole;
temp.numer = (numer * obj2.denom) + (obj2.numer * denom);
temp.denom = denom * obj2.denom;

temp.reduce();
return temp;
}

fracpri fracpri::operat or -(fracpri &obj2){
fracpri temp;

// they are converted to improper fractions
// because the second fraction may be larger than the
// first and may cause the final fraction to be negative
// and the whole number to be positive

temp.whole = whole - obj2.whole;
temp.numer = ((whole * denom + numer)*obj2.den om) - ((obj2.whole *
obj2.denom + obj2.numer)*den om);
temp.denom = denom * obj2.denom;

temp.reduce();
return temp;
}

fracpri fracpri::operat or *(fracpri &obj2){
fracpri temp;

// convert both to improper
temp.numer = ((whole * denom) + numer) * ((obj2.whole * obj2.denom)
+ obj2.numer);

// the denominator is the two fractions denominators multiplied
together
temp.denom = denom * obj2.denom;

temp.whole = temp.numer/temp.denom;

temp.numer %= temp.denom;

temp.reduce();
return temp;
}
fracpri fracpri::operat or /(fracpri &obj2){
fracpri temp;


// convert both to improper
temp.numer = ((whole * denom) + numer) * obj2.denom;

temp.denom = denom * ((obj2.whole * obj2.denom) + obj2.numer);

temp.whole = temp.numer/temp.denom;

temp.numer %= temp.denom;

temp.reduce();

return temp;
}

fracpri operator+(int i, fracpri &obj2){
fracpri temp;

temp.whole = i + obj2.whole;
temp.numer = obj2.numer;
temp.denom = obj2.denom;


return temp;
}

fracpri operator+(fracp ri &obj1, int i){
fracpri temp;

temp.whole = obj1.whole + i;
temp.numer = obj1.numer;
temp.denom = obj1.denom;


return temp;
}

bool fracpri::operat or<(fracpri &obj2){

// each fractions is multiplied by the other fraction's denominator
so that both fractions
// numerators are the result of a common denominator, which is denom
*obj2.denom
// each fraction's orignal denominator cancels out when the common
denominator is multiplied
// to each of the fractions.

if(((whole * denom + numer)*obj2.den om) < ((obj2.whole * obj2.denom
+ numer)* denom))
return true;
else
return false;
}

fracpri& fracpri::operat or +=(fracpri &obj2){
whole += obj2.whole; // same as whole = whole + obj2.whole
numer = (numer * obj2.denom) + (obj2.numer * denom);
denom *= obj2.denom; // same as denom = denom * obj2.denom

reduce();
return *this;
}

istream& operator >>(istream &in, fracpri &obj2){
char ch;
in >> obj2.whole >> ch >> obj2.numer >> ch >> obj2.denom;

if(obj2.denom == 0)
obj2.denom = 1;

obj2.reduce();
return in;
}

ostream& operator <<(ostream &out, fracpri const& obj2){
out << obj2.whole << ' ' << obj2.numer ;
out << '/' << obj2.denom;

return out;
}

fracpri::operat or float(){ // class type to float type

// float cast is needed because int divided by int results in int
and not float
// however float divided by int equals a float
float f = float(numer)/denom;

return f + whole;
}

void fracpri::reduce (){
while(numer >= denom){
whole++;
numer -= denom;
}
int count = numer * denom; // a common denominator
for(; count > 1; count--){
if((numer % count == 0) && (denom % count == 0)){ // if count fits
into both
numer /= count; // numerator and denominator without a
remainder,
denom /= count; // then divide both of them by that
// and then continue to count down
}
}
}



int main(){

int choice = 0; // used check if the calculations
should be reran
fracpri stock1; // a. 0 argument constructor
fracpri stock2(5,1,3); // b. 3 argument constructor
float decimal = 0.0;

fracpri stock[3];

do{
cout << "Enter two stock prices that will then be used" << endl;
cout << "with all the functions of this program" << endl;

cout << "Enter a stock price using getFraction" << endl;
stock[0].getFraction(); // c. getfraction function
cout << "Enter a stock price using >> operator" << endl;
cout << "In the form of whole-numerator-denominator" << endl;
cin >> stock[1]; // l. the >> operator

cout << "Using addfracts function and << operator" << endl;
stock[2].addfracts(stoc k[0], stock[1]); //e. addfracts
cout << stock[2] << endl;;

cout << "Using + operator, overloaded, and showFraction() function"
<< endl;
stock[2] = stock[0] + stock[1]; // f. + , -, *, and / overloaded
stock[2].showFraction() ;

cout << "Using - operator, overloaded, and << overloaded" << endl;
stock[2] = stock[0] - stock[1]; // f.
cout << stock[2] << endl;

cout << "Using * operator, overloaded, and << overloaded" << endl;
stock[2] = stock[0] * stock[1]; // f.
cout << stock[2] << endl;

cout << "Using / operator, overloaded, and showFraction() function"
<< endl;
stock[2] = stock[0] / stock[1]; // f.
stock[2].showFraction() ;

cout << "Using constant +, overloaed, and << overloaded" << endl;
stock[2] = 5 + stock[0]; // g. 5 + fracpri
cout << stock[2] << endl;


cout << "Using + constant, overloaded, and showFraction() function"
<< endl;
stock[2] = stock[0] + 5; // g. fracpri + 5
stock[2].showFraction() ;


cout << "Using the < operator, overloaded." << endl;
if((stock[0] < stock[1]) == 1) // h. < overloaded
cout << "True" << endl;
else if((stock[0] < stock[1]) == 0)
cout << "False" << endl;


cout << "The += operator overloaded and using the showFraction()
function" << endl;
stock[0] += stock[1]; // k. += overloaded
stock[0].showFraction() ;

choice = 0; // reset choice to 0 so the menu pops up and
the previous stuff does
// not continue

while(choice != 3 && choice != 4){

cout << "Enter a choice" << endl;
cout << "Press 1 to convert decimal to fraction using a class
constructor" << endl;
cout << "Press 2 to convert fraction to a decimal using float
overloaded, a cast overload" << endl;
cout << "Press 3 to rerun the program" << endl;
cout << "Press 4 to quit the program" << endl;
cin >> choice;

if(choice == 1){
cout << "Enter a decimal" << endl;
cin >> decimal;
fracpri stock5 = decimal; // m. decimal to fraction
// fracpri stock5 = decimal
uses a constructor to convert
// and it is the same as doing
fracpri stock5(decimal)

// the fracpri stock5 is
destroyed when the if(choice ==1)
// scope gets destroyed, so
the constructor always works.
cout << "In fraction form the decimal is" << endl;
cout << stock5 << endl;
cin.ignore(INT_ MAX, '\n');
}

else if(choice == 2){
cout << "Enter a fraction" << endl;
cin >> stock[2];
decimal = stock[2]; // m. fraction to decimal
cout << "In decimal form the fraction is" << endl;
cout << decimal << endl;
cin.ignore(INT_ MAX, '\n');
}
else if(choice != 3 && choice != 4){
cout << "Invalid entry, please try agian" << endl;
}
}

}while(choice != 4);


return 0;




}

[Rolf Magnus]

Re: what'ch think of my fraction program, is it correct and good and efficient?

John Cho wrote:
[color=blue]
> // CHO, JOHN
>
> #include<iostre am>
>
>
> class fracpri{[/color]

I've seen better names than "fracpri" :-)
[color=blue]
> int whole;
> int numer;
> int denom;
>
> public:
> // constructors:
>
> fracpri();
> fracpri(int w, int n, int d);
> fracpri(float f); // float to class constructor
>
> // member functions to show output and get input
> void getFraction();[/color]

That function doesn't modify the object, so make it const:

void getFraction() const;
[color=blue]
> void showFraction();[/color]

void showFraction() const;

Actually, your whole class isn't written with const correctness in mind.
[color=blue]
> // function and operators to do artimathetic
> fracpri addfracts(fracp ri &obj1, fracpri &obj2);[/color]

That should rather look like:

fracpri& addfracts(const fracpri &obj1, const fracpri &obj2);
[color=blue]
> fracpri operator+(fracp ri &obj2);[/color]

fracpri operator+(const fracpri &obj2);

same for the others.
[color=blue]
> fracpri operator-(fracpri &obj2);
> fracpri operator*(fracp ri &obj2);
> fracpri operator/(fracpri &obj2);[/color]

You could make the above operators non-members.
[color=blue]
>
> // adding with constants
> friend fracpri operator +(int i, fracpri &obj2);
> friend fracpri operator +(fracpri &obj1, int i);
>
> // the compare operator, <
> bool operator<(fracp ri &obj2);[/color]

That should be a non-member too.
[color=blue]
>
> // the += operator
> fracpri& operator+=(frac pri &obj2);[/color]

fracpri& operator+=(cons t fracpri &obj2);
[color=blue]
> // overloading the input and output, >> and << operators.
> friend std::istream& operator>> (std::istream& in, fracpri& obj2);[/color]

friend std::istream& operator>> (std::istream& in, const fracpri& obj2);
[color=blue]
> // std:: is used because
> friend std::ostream& operator<< (std::ostream& out, fracpri const&
> obj2);
> // namespace std is after it
>
>
> void reduce();
>
> operator float(); // class to float conversion[/color]

operator float() const; // class to float conversion
[color=blue]
> };
>
> using namespace std; // namespace needs to be after any use of
> friend
> // or else msvc++ 6.0 gives errors[/color]

I'd leave the using out completely here.
[color=blue]
> fracpri::fracpr i(){
> whole = 0;
> numer = 0;
> denom = 1; // it is set as 1 so the adding fractions
> works with a
> // common denominator and not 0
> }[/color]

Prefer initialization over assignment:

fracpri::fracpr i()
: whole(0),
numer(0),
denom(1)
{
}
[color=blue]
> fracpri::fracpr i(int w, int n, int d){
> whole = w;
> numer = n;
> denom = d;
> if(denom == 0)
> denom = 1;
> reduce();
> }[/color]

fracpri::fracpr i(int w, int n, int d)
: whole(w),
numer(n),
denom(d ? d : 1)
{
reduce();
}
[color=blue]
> fracpri::fracpr i(float f){ //float type to class type constructor
>
> f += 0.0078125; // .0078125 so numerator is rounded up later[/color]

I'd add a static const class member variable for that magic constant.
And you should generally prefer double over float if you don't have a
very good reason not to.
Oh, and what happens if f is negative? Does your rounding still work as
expected?
[color=blue]
> whole = int(f);
> float flofrac = f - whole; // the fraction part of the number
> numer = (flofrac * 64); // .0078125 * 64 = .5 ; so rounded up
> numerator
> // if orignal fraction
> denom = 64;
> reduce();
> }
>
> void fracpri::getFra ction(){
> char ch;
> cout << "Enter a stock price in the form of " << endl;
> cout << "wholenumbe r-numerator-denominator" << endl;
> cin >> whole >> ch >> numer >> ch >> denom;
> if(denom == 0)
> denom = 1; // fix the denom if it is 0
> reduce();
> }
>
> void fracpri::showFr action(){
>
> cout << "The stock price is: " << endl;
> cout << whole << ' ' << numer << '/' << denom << endl;
>
> }[/color]

Better avoid putting user interaction into such "low level" classes.
Each class should have only one task, and user interaction would be a
second task.
[color=blue]
> fracpri fracpri::addfra cts(fracpri &obj1, fracpri &obj2){
> whole = obj1.whole + obj2.whole;
> numer = (obj1.numer * obj2.denom) + (obj2.numer * obj1.denom);
> denom = obj1.denom * obj2.denom;
>
> reduce();
> return *this;
> }
>
> fracpri fracpri::operat or +(fracpri &obj2){
> fracpri temp; // temp is here because *this would modify
> // the first object or a in
> // c = a + b
> temp.whole = whole + obj2.whole;
> temp.numer = (numer * obj2.denom) + (obj2.numer * denom);
> temp.denom = denom * obj2.denom;
>
> temp.reduce();
> return temp;
> }[/color]

Don't duplicate code. Just re-use operator+=, which already does the
job:

fracpri fracpri::operat or +(const fracpri &obj2)
{
// create a temporary copy, add obj2 to it and return it
return fracpri(*this) += obj2;
}
[color=blue]
> fracpri fracpri::operat or -(fracpri &obj2){
> fracpri temp;
>
> // they are converted to improper fractions
> // because the second fraction may be larger than the
> // first and may cause the final fraction to be negative
> // and the whole number to be positive
>
> temp.whole = whole - obj2.whole;
> temp.numer = ((whole * denom + numer)*obj2.den om) - ((obj2.whole *
> obj2.denom + obj2.numer)*den om);
> temp.denom = denom * obj2.denom;
>
> temp.reduce();
> return temp;
> }
>
> fracpri fracpri::operat or *(fracpri &obj2){
> fracpri temp;
>
> // convert both to improper
> temp.numer = ((whole * denom) + numer) * ((obj2.whole * obj2.denom)
> + obj2.numer);
>
> // the denominator is the two fractions denominators multiplied
> together
> temp.denom = denom * obj2.denom;
>
> temp.whole = temp.numer/temp.denom;
>
> temp.numer %= temp.denom;
>
> temp.reduce();
> return temp;
> }
> fracpri fracpri::operat or /(fracpri &obj2){
> fracpri temp;
>
>
> // convert both to improper
> temp.numer = ((whole * denom) + numer) * obj2.denom;
>
> temp.denom = denom * ((obj2.whole * obj2.denom) + obj2.numer);
>
> temp.whole = temp.numer/temp.denom;
>
> temp.numer %= temp.denom;
>
> temp.reduce();
>
> return temp;
> }
>
> fracpri operator+(int i, fracpri &obj2){
> fracpri temp;
>
> temp.whole = i + obj2.whole;
> temp.numer = obj2.numer;
> temp.denom = obj2.denom;
>
>
> return temp;
> }
>
> fracpri operator+(fracp ri &obj1, int i){
> fracpri temp;
>
> temp.whole = obj1.whole + i;
> temp.numer = obj1.numer;
> temp.denom = obj1.denom;
>
>
> return temp;
> }
>
> bool fracpri::operat or<(fracpri &obj2){
>
> // each fractions is multiplied by the other fraction's denominator
> so that both fractions
> // numerators are the result of a common denominator, which is denom
> *obj2.denom
> // each fraction's orignal denominator cancels out when the common
> denominator is multiplied
> // to each of the fractions.
>
> if(((whole * denom + numer)*obj2.den om) < ((obj2.whole * obj2.denom
> + numer)* denom))
> return true;
> else
> return false;
> }
>
> fracpri& fracpri::operat or +=(fracpri &obj2){
> whole += obj2.whole; // same as whole = whole + obj2.whole
> numer = (numer * obj2.denom) + (obj2.numer * denom);
> denom *= obj2.denom; // same as denom = denom * obj2.denom
>
> reduce();
> return *this;
> }
>
> istream& operator >>(istream &in, fracpri &obj2){
> char ch;
> in >> obj2.whole >> ch >> obj2.numer >> ch >> obj2.denom;[/color]

Your first '>> ch' will screw things up. Below, you only write a
whitespace into the stream, and whitepsace is ignored when reading.
[color=blue]
> if(obj2.denom == 0)
> obj2.denom = 1;
>
> obj2.reduce();
> return in;
> }
>
> ostream& operator <<(ostream &out, fracpri const& obj2){
> out << obj2.whole << ' ' << obj2.numer ;
> out << '/' << obj2.denom;
>
> return out;
> }
>
> fracpri::operat or float(){ // class type to float type
>
> // float cast is needed because int divided by int results in int
> and not float
> // however float divided by int equals a float
> float f = float(numer)/denom;
>
> return f + whole;
> }
>
> void fracpri::reduce (){
> while(numer >= denom){
> whole++;
> numer -= denom;
> }
> int count = numer * denom; // a common denominator
> for(; count > 1; count--){
> if((numer % count == 0) && (denom % count == 0)){ // if count fits
> into both
> numer /= count; // numerator and denominator without a
> remainder,
> denom /= count; // then divide both of them by that
> // and then continue to count down
> }
> }
> }
>
>
>
> int main(){
>
> int choice = 0; // used check if the calculations
> should be reran
> fracpri stock1; // a. 0 argument constructor
> fracpri stock2(5,1,3); // b. 3 argument constructor
> float decimal = 0.0;
>
> fracpri stock[3];
>
> do{
> cout << "Enter two stock prices that will then be used" << endl;
> cout << "with all the functions of this program" << endl;
>
> cout << "Enter a stock price using getFraction" << endl;
> stock[0].getFraction(); // c. getfraction function
> cout << "Enter a stock price using >> operator" << endl;
> cout << "In the form of whole-numerator-denominator" << endl;
> cin >> stock[1]; // l. the >> operator
>
> cout << "Using addfracts function and << operator" << endl;
> stock[2].addfracts(stoc k[0], stock[1]); //e. addfracts
> cout << stock[2] << endl;;
>
> cout << "Using + operator, overloaded, and showFraction() function"
> << endl;
> stock[2] = stock[0] + stock[1]; // f. + , -, *, and / overloaded
> stock[2].showFraction() ;
>
> cout << "Using - operator, overloaded, and << overloaded" << endl;
> stock[2] = stock[0] - stock[1]; // f.
> cout << stock[2] << endl;
>
> cout << "Using * operator, overloaded, and << overloaded" << endl;
> stock[2] = stock[0] * stock[1]; // f.
> cout << stock[2] << endl;
>
> cout << "Using / operator, overloaded, and showFraction() function"
> << endl;
> stock[2] = stock[0] / stock[1]; // f.
> stock[2].showFraction() ;
>
> cout << "Using constant +, overloaed, and << overloaded" << endl;
> stock[2] = 5 + stock[0]; // g. 5 + fracpri
> cout << stock[2] << endl;
>
>
> cout << "Using + constant, overloaded, and showFraction() function"
> << endl;
> stock[2] = stock[0] + 5; // g. fracpri + 5
> stock[2].showFraction() ;
>
>
> cout << "Using the < operator, overloaded." << endl;
> if((stock[0] < stock[1]) == 1) // h. < overloaded
> cout << "True" << endl;
> else if((stock[0] < stock[1]) == 0)
> cout << "False" << endl;
>
>
> cout << "The += operator overloaded and using the showFraction()
> function" << endl;
> stock[0] += stock[1]; // k. += overloaded
> stock[0].showFraction() ;
>
> choice = 0; // reset choice to 0 so the menu pops up and
> the previous stuff does
> // not continue
>
> while(choice != 3 && choice != 4){
>
> cout << "Enter a choice" << endl;
> cout << "Press 1 to convert decimal to fraction using a class
> constructor" << endl;
> cout << "Press 2 to convert fraction to a decimal using float
> overloaded, a cast overload" << endl;
> cout << "Press 3 to rerun the program" << endl;
> cout << "Press 4 to quit the program" << endl;
> cin >> choice;
>
> if(choice == 1){
> cout << "Enter a decimal" << endl;
> cin >> decimal;
> fracpri stock5 = decimal; // m. decimal to fraction
> // fracpri stock5 = decimal
> uses a constructor to convert
> // and it is the same as doing
> fracpri stock5(decimal)
>
> // the fracpri stock5 is
> destroyed when the if(choice ==1)
> // scope gets destroyed, so
> the constructor always works.
> cout << "In fraction form the decimal is" << endl;
> cout << stock5 << endl;
> cin.ignore(INT_ MAX, '\n');
> }
>
> else if(choice == 2){
> cout << "Enter a fraction" << endl;
> cin >> stock[2];
> decimal = stock[2]; // m. fraction to decimal
> cout << "In decimal form the fraction is" << endl;
> cout << decimal << endl;
> cin.ignore(INT_ MAX, '\n');
> }
> else if(choice != 3 && choice != 4){
> cout << "Invalid entry, please try agian" << endl;
> }
> }
>
> }while(choice != 4);
>
>
> return 0;
>
>
>
>
> }[/color]

--
Kyle: "Hey, Stan, now that Terrance & Phillip has been taken off the
air, what
are we gonna do for entertainment?"
Stan: "I don't know. We, we could start breathing gas fumes."
Cartman: "My uncle says that smoking crack is kinda cool"
Kyle: "Hey, why don't we watch some of those porno movie thingies?"

[Victor Bazarov]

Re: what'ch think of my fraction program, is it correct and good and efficient?

"Rolf Magnus" <ramagnus@t-online.de> wrote...[color=blue]
> John Cho wrote:
>[color=green]
> > // CHO, JOHN
> >
> > #include<iostre am>
> >
> >
> > class fracpri{[/color]
>
> I've seen better names than "fracpri" :-)
>[color=green]
> > int whole;
> > int numer;
> > int denom;
> >
> > public:
> > // constructors:
> >
> > fracpri();
> > fracpri(int w, int n, int d);
> > fracpri(float f); // float to class constructor
> >
> > // member functions to show output and get input
> > void getFraction();[/color]
>
> That function doesn't modify the object, so make it const:
>
> void getFraction() const;[/color]

Hmm.. Somehow I suspect that this one actually modifies the object.
[color=blue]
>[color=green]
> > void showFraction();[/color]
>
> void showFraction() const;
>
> Actually, your whole class isn't written with const correctness in mind.
>[color=green]
> > // function and operators to do artimathetic
> > fracpri addfracts(fracp ri &obj1, fracpri &obj2);[/color]
>
> That should rather look like:
>
> fracpri& addfracts(const fracpri &obj1, const fracpri &obj2);[/color]

Shouldn't it actually be static?
[color=blue]
>[color=green]
> > fracpri operator+(fracp ri &obj2);[/color]
>
> fracpri operator+(const fracpri &obj2);[/color]

fracpri operator+(const fracpri &obj2) const;
[color=blue]
>
> same for the others.
>[color=green]
> > fracpri operator-(fracpri &obj2);
> > fracpri operator*(fracp ri &obj2);
> > fracpri operator/(fracpri &obj2);[/color]
>
> You could make the above operators non-members.
>[color=green]
> >
> > // adding with constants
> > friend fracpri operator +(int i, fracpri &obj2);
> > friend fracpri operator +(fracpri &obj1, int i);
> >
> > // the compare operator, <
> > bool operator<(fracp ri &obj2);[/color]
>
> That should be a non-member too.[/color]

Or at least made 'const'.
[color=blue]
>[color=green]
> >
> > // the += operator
> > fracpri& operator+=(frac pri &obj2);[/color]
>
> fracpri& operator+=(cons t fracpri &obj2);
>[color=green]
> > // overloading the input and output, >> and << operators.
> > friend std::istream& operator>> (std::istream& in, fracpri& obj2);[/color]
>
> friend std::istream& operator>> (std::istream& in, const fracpri& obj2);
>[color=green]
> > // std:: is used because
> > friend std::ostream& operator<< (std::ostream& out, fracpri const&
> > obj2);
> > // namespace std is after it
> >
> >
> > void reduce();
> >
> > operator float(); // class to float conversion[/color]
>
> operator float() const; // class to float conversion
>[color=green]
> > };
> >
> > using namespace std; // namespace needs to be after any use of
> > friend
> > // or else msvc++ 6.0 gives errors[/color]
>
> I'd leave the using out completely here.
>[color=green]
> > fracpri::fracpr i(){
> > whole = 0;
> > numer = 0;
> > denom = 1; // it is set as 1 so the adding fractions
> > works with a
> > // common denominator and not 0
> > }[/color]
>
> Prefer initialization over assignment:[/color]

Prefer referring to FAQ. It contains explanation why initialisation
should be preferred.
[color=blue]
>
> fracpri::fracpr i()
> : whole(0),
> numer(0),
> denom(1)
> {
> }
>
> [...other fine advice omitted for brevity...][/color]

V

[John Cho]

Re: what'ch think of my fraction program, is it correct and good and efficient?

> Prefer referring to FAQ. It contains explanation why initialisation[color=blue]
> should be preferred.
>[/color]


assignment should be fine as long as i am not trying to initialize const
data members or reference data members

[color=blue][color=green]
>> fracpri& addfracts(const fracpri &obj1, const fracpri &obj2);[/color][/color]
[color=blue]
>Shouldn't it actually be static?[/color]

what do you mean by static?


i am still trying to figure out why you return by reference if you do nto
use a this pointer

[John Cho]

Re: what'ch think of my fraction program, is it correct and good and efficient?

"Victor Bazarov" <v.Abazarov@com Acast.net> wrote in news:byT0c.9605 9
$Xp.427249@attb i_s54:
[color=blue][color=green][color=darkred]
>> > fracpri operator*(fracp ri &obj2);
>> > fracpri operator/(fracpri &obj2);[/color]
>>
>> You could make the above operators non-members.[/color][/color]

please tell me why you prefer gobal over methods?

[Keith H Duggar]

Re: what'ch think of my fraction program, is it correct and good and efficient?

Wow what a lot of work to try and create a new wheel.

First, boost.org has a fairly nice rational number class that you
should at least take a look at. Second, you don't necessarily need a
whole number portion. However, both of those comments are matters of
context so you may or may not want to listen to them.

However, there is one thing that you definitely should do and that is
implement Euclids GCD algorithm ro reduce instead of this naive one:

[color=blue]
> void fracpri::reduce (){
> while(numer >= denom){
> whole++;
> numer -= denom;
> }
> int count = numer * denom; // a common denominator
> for(; count > 1; count--){
> if((numer % count == 0) && (denom % count == 0)){ // if count fits
> into both
> numer /= count; // numerator and denominator without a
> remainder,
> denom /= count; // then divide both of them by that
> // and then continue to count down
> }
> }
> }[/color]

The boost rational number class switched over to Euclid's at some
point since the last time I looked so you can find an implementation
there.

[John Harrison]

Re: what'ch think of my fraction program, is it correct and good and efficient?


"John Cho" <johncho@johnch o.us> wrote in message
news:Xns949FE53 1438B1spockieve rizonnet@199.45 .49.11...[color=blue]
> "Victor Bazarov" <v.Abazarov@com Acast.net> wrote in news:byT0c.9605 9
> $Xp.427249@attb i_s54:
>[color=green][color=darkred]
> >> > fracpri operator*(fracp ri &obj2);
> >> > fracpri operator/(fracpri &obj2);
> >>
> >> You could make the above operators non-members.[/color][/color]
>
> please tell me why you prefer gobal over methods?[/color]

If you have a global then this will compile

fracpri f;
2.0 * f;
f * 2.0;

Because the fracpri(float) constructor will be implicitly invoked on 2.0.

If you have a method, the first multiplication will not. Basically C++ has
different rules on method parameters and the method object. Since you want
the same rules for all the parameters of *, you should use a global
function.

john

[John Harrison]

Re: what'ch think of my fraction program, is it correct and good and efficient?

>[color=blue]
> void fracpri::reduce (){
> while(numer >= denom){
> whole++;
> numer -= denom;
> }
> int count = numer * denom; // a common denominator
> for(; count > 1; count--){
> if((numer % count == 0) && (denom % count == 0)){ // if count fits
> into both
> numer /= count; // numerator and denominator without a
> remainder,
> denom /= count; // then divide both of them by that
> // and then continue to count down
> }
> }
> }
>[/color]

This is inefficient, since you are risking integer overflow when you say
numer * denom. Instead you should use a greatest common divisor algorithm
(gcd).

void fracpri::reduce (){
while(numer >= denom){
whole++;
numer -= denom;
}
int d = gcd(numer, denom);
numer /= d;
denom /= d;
}

The gcd algorithm can be found in books or on the internet. It's only the
oldest algorithm known to mankind.

john

[Rolf Magnus]

Re: what'ch think of my fraction program, is it correct and good and efficient?

Victor Bazarov wrote:
[color=blue][color=green][color=darkred]
>> > // member functions to show output and get input
>> > void getFraction();[/color]
>>
>> That function doesn't modify the object, so make it const:
>>
>> void getFraction() const;[/color]
>
> Hmm.. Somehow I suspect that this one actually modifies the object.[/color]

Oh, right. It reduces...
[color=blue][color=green][color=darkred]
>> > // function and operators to do artimathetic
>> > fracpri addfracts(fracp ri &obj1, fracpri &obj2);[/color]
>>
>> That should rather look like:
>>
>> fracpri& addfracts(const fracpri &obj1, const fracpri &obj2);[/color]
>
> Shouldn't it actually be static?[/color]

Nope. Look into the implementation. It makes the 'this' object the
result of the addition of obj1 and obj2.
[color=blue][color=green][color=darkred]
>> > // the compare operator, <
>> > bool operator<(fracp ri &obj2);[/color]
>>
>> That should be a non-member too.[/color]
>
> Or at least made 'const'.[/color]

Yes, but only if it stays as a member ;-)
[color=blue][color=green]
>> Prefer initialization over assignment:[/color]
>
> Prefer referring to FAQ. It contains explanation why initialisation
> should be preferred.[/color]

Ok, I'll try to do that next time.

--
"computer games don't affect kids. I mean if pac man affected us as
kids, we'd all run around in a darkened room munching pills"

[Rolf Magnus]

Re: what'ch think of my fraction program, is it correct and good and efficient?

John Cho wrote:
[color=blue][color=green]
>> Prefer referring to FAQ. It contains explanation why initialisation
>> should be preferred.[/color]
>
> assignment should be fine as long as i am not trying to initialize
> const data members or reference data members[/color]

And as long as your members aren't class instances. You're right in that
it typically makes no difference in the generated code for built-in
types. But why make an exception for only non-const, non-reference,
non-class types? Just do it the same way for all. It's called
consistency.
[color=blue][color=green][color=darkred]
>>> fracpri& addfracts(const fracpri &obj1, const fracpri &obj2);[/color][/color]
>[color=green]
>>Shouldn't it actually be static?[/color]
>
> what do you mean by static?[/color]

That you can call the function without an object, but since your
function writes the result to the 'this' objec, it should actually not
be static.
[color=blue]
> i am still trying to figure out why you return by reference if you do
> nto use a this pointer[/color]

Are you talking about the above function being static, or is it a
general question? Returning by reference isn't connected to having a
this pointer. You just have to make sure that the object reffered to
stays alive beyond the function call.