By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
431,934 Members | 1,677 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 431,934 IT Pros & Developers. It's quick & easy.

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

P: n/a
// CHO, JOHN

#include<iostream>
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(fracpri &obj1, fracpri &obj2);
fracpri operator+(fracpri &obj2);
fracpri operator-(fracpri &obj2);
fracpri operator*(fracpri &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<(fracpri &obj2);

// the += operator
fracpri& operator+=(fracpri &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::fracpri(){
whole = 0;
numer = 0;
denom = 1; // it is set as 1 so the adding fractions
works with a
// common denominator and not 0
}

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

fracpri::fracpri(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::getFraction(){
char ch;
cout << "Enter a stock price in the form of " << endl;
cout << "wholenumber-numerator-denominator" << endl;
cin >> whole >> ch >> numer >> ch >> denom;
if(denom == 0)
denom = 1; // fix the denom if it is 0
reduce();
}

void fracpri::showFraction(){

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

}

fracpri fracpri::addfracts(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::operator +(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::operator -(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.denom) - ((obj2.whole *
obj2.denom + obj2.numer)*denom);
temp.denom = denom * obj2.denom;

temp.reduce();
return temp;
}

fracpri fracpri::operator *(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::operator /(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+(fracpri &obj1, int i){
fracpri temp;

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

bool fracpri::operator<(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.denom) < ((obj2.whole * obj2.denom
+ numer)* denom))
return true;
else
return false;
}

fracpri& fracpri::operator +=(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::operator 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(stock[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;


}
Jul 22 '05 #1
Share this Question
Share on Google+
9 Replies


P: n/a
John Cho wrote:
// CHO, JOHN

#include<iostream>
class fracpri{
I've seen better names than "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();
That function doesn't modify the object, so make it const:

void getFraction() const;
void showFraction();
void showFraction() const;

Actually, your whole class isn't written with const correctness in mind.
// function and operators to do artimathetic
fracpri addfracts(fracpri &obj1, fracpri &obj2);
That should rather look like:

fracpri& addfracts(const fracpri &obj1, const fracpri &obj2);
fracpri operator+(fracpri &obj2);
fracpri operator+(const fracpri &obj2);

same for the others.
fracpri operator-(fracpri &obj2);
fracpri operator*(fracpri &obj2);
fracpri operator/(fracpri &obj2);
You could make the above operators non-members.

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

// the compare operator, <
bool operator<(fracpri &obj2);
That should be a non-member too.

// the += operator
fracpri& operator+=(fracpri &obj2);
fracpri& operator+=(const fracpri &obj2);
// overloading the input and output, >> and << operators.
friend std::istream& operator>> (std::istream& in, fracpri& obj2);
friend std::istream& operator>> (std::istream& in, const 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
operator float() const; // class to float conversion
};

using namespace std; // namespace needs to be after any use of
friend
// or else msvc++ 6.0 gives errors
I'd leave the using out completely here.
fracpri::fracpri(){
whole = 0;
numer = 0;
denom = 1; // it is set as 1 so the adding fractions
works with a
// common denominator and not 0
}
Prefer initialization over assignment:

fracpri::fracpri()
: whole(0),
numer(0),
denom(1)
{
}
fracpri::fracpri(int w, int n, int d){
whole = w;
numer = n;
denom = d;
if(denom == 0)
denom = 1;
reduce();
}
fracpri::fracpri(int w, int n, int d)
: whole(w),
numer(n),
denom(d ? d : 1)
{
reduce();
}
fracpri::fracpri(float f){ //float type to class type constructor

f += 0.0078125; // .0078125 so numerator is rounded up later
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?
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::getFraction(){
char ch;
cout << "Enter a stock price in the form of " << endl;
cout << "wholenumber-numerator-denominator" << endl;
cin >> whole >> ch >> numer >> ch >> denom;
if(denom == 0)
denom = 1; // fix the denom if it is 0
reduce();
}

void fracpri::showFraction(){

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

}
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.
fracpri fracpri::addfracts(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::operator +(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;
}
Don't duplicate code. Just re-use operator+=, which already does the
job:

fracpri fracpri::operator +(const fracpri &obj2)
{
// create a temporary copy, add obj2 to it and return it
return fracpri(*this) += obj2;
}
fracpri fracpri::operator -(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.denom) - ((obj2.whole *
obj2.denom + obj2.numer)*denom);
temp.denom = denom * obj2.denom;

temp.reduce();
return temp;
}

fracpri fracpri::operator *(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::operator /(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+(fracpri &obj1, int i){
fracpri temp;

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

bool fracpri::operator<(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.denom) < ((obj2.whole * obj2.denom
+ numer)* denom))
return true;
else
return false;
}

fracpri& fracpri::operator +=(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;
Your first '>> ch' will screw things up. Below, you only write a
whitespace into the stream, and whitepsace is ignored when reading.
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::operator 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(stock[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;


}


--
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?"

Jul 22 '05 #2

P: n/a
"Rolf Magnus" <ra******@t-online.de> wrote...
John Cho wrote:
// CHO, JOHN

#include<iostream>
class fracpri{
I've seen better names than "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();


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

void getFraction() const;


Hmm.. Somehow I suspect that this one actually modifies the object.
void showFraction();
void showFraction() const;

Actually, your whole class isn't written with const correctness in mind.
// function and operators to do artimathetic
fracpri addfracts(fracpri &obj1, fracpri &obj2);


That should rather look like:

fracpri& addfracts(const fracpri &obj1, const fracpri &obj2);


Shouldn't it actually be static?
fracpri operator+(fracpri &obj2);
fracpri operator+(const fracpri &obj2);


fracpri operator+(const fracpri &obj2) const;

same for the others.
fracpri operator-(fracpri &obj2);
fracpri operator*(fracpri &obj2);
fracpri operator/(fracpri &obj2);
You could make the above operators non-members.

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

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


That should be a non-member too.


Or at least made 'const'.

// the += operator
fracpri& operator+=(fracpri &obj2);
fracpri& operator+=(const fracpri &obj2);
// overloading the input and output, >> and << operators.
friend std::istream& operator>> (std::istream& in, fracpri& obj2);


friend std::istream& operator>> (std::istream& in, const 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


operator float() const; // class to float conversion
};

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


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


Prefer initialization over assignment:


Prefer referring to FAQ. It contains explanation why initialisation
should be preferred.

fracpri::fracpri()
: whole(0),
numer(0),
denom(1)
{
}

[...other fine advice omitted for brevity...]


V
Jul 22 '05 #3

P: n/a
> Prefer referring to FAQ. It contains explanation why initialisation
should be preferred.
assignment should be fine as long as i am not trying to initialize const
data members or reference data members

fracpri& addfracts(const fracpri &obj1, const fracpri &obj2);

Shouldn't it actually be static?


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
Jul 22 '05 #4

P: n/a
"Victor Bazarov" <v.********@comAcast.net> wrote in news:byT0c.96059
$Xp.427249@attbi_s54:
> fracpri operator*(fracpri &obj2);
> fracpri operator/(fracpri &obj2);


You could make the above operators non-members.


please tell me why you prefer gobal over methods?
Jul 22 '05 #5

P: n/a
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:

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
}
}
}


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.
Jul 22 '05 #6

P: n/a

"John Cho" <jo*****@johncho.us> wrote in message
news:Xn*******************************@199.45.49.1 1...
"Victor Bazarov" <v.********@comAcast.net> wrote in news:byT0c.96059
$Xp.427249@attbi_s54:
> fracpri operator*(fracpri &obj2);
> fracpri operator/(fracpri &obj2);

You could make the above operators non-members.


please tell me why you prefer gobal over methods?


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
Jul 22 '05 #7

P: n/a
>
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
}
}
}


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
Jul 22 '05 #8

P: n/a
Victor Bazarov wrote:
> // member functions to show output and get input
> void getFraction();


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

void getFraction() const;


Hmm.. Somehow I suspect that this one actually modifies the object.


Oh, right. It reduces...
> // function and operators to do artimathetic
> fracpri addfracts(fracpri &obj1, fracpri &obj2);


That should rather look like:

fracpri& addfracts(const fracpri &obj1, const fracpri &obj2);


Shouldn't it actually be static?


Nope. Look into the implementation. It makes the 'this' object the
result of the addition of obj1 and obj2.
> // the compare operator, <
> bool operator<(fracpri &obj2);


That should be a non-member too.


Or at least made 'const'.


Yes, but only if it stays as a member ;-)
Prefer initialization over assignment:


Prefer referring to FAQ. It contains explanation why initialisation
should be preferred.


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"

Jul 22 '05 #9

P: n/a
John Cho wrote:
Prefer referring to FAQ. It contains explanation why initialisation
should be preferred.
assignment should be fine as long as i am not trying to initialize
const data members or reference data members


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.
fracpri& addfracts(const fracpri &obj1, const fracpri &obj2);

Shouldn't it actually be static?


what do you mean by static?


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.
i am still trying to figure out why you return by reference if you do
nto use a this pointer


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.

Jul 22 '05 #10

This discussion thread is closed

Replies have been disabled for this discussion.