Jump to content
  • Advertisement
Sign in to follow this  
MubasharIqbal

plz correct the code

This topic is 4269 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

i have wriiten the following code but it produces a error plz correct the code: #include<iostream.h> #include<conio.h> #include<stdlib.h> class Blood_Pressure{ private: float upper; float lower; public: Blood_Pressure() { upper=lower=0.0; } ~Blood_Pressure() { } void setupper(const float _upper) { upper=_upper; } void setlower(const float _lower) { lower=_lower; } float getupper(void) { return upper; } float getlower(void) { return lower; } }; class Patient{ private: char name[20]; Blood_Pressure BP; public: Patient() { char _name[20]; cout <<"enter your name.."; cin>>name; float _upper,_lower; cout<<"enter upper limit.."; cin>>_upper; BP.setupper(_upper); cout<<"enter lower limit.."; cin>>_lower; BP.setlower(_lower); CalculateReport(); } ~Patient() { } void setname(const char _name[20]) { strcpy(name,_name); } char* getname(void) { return name; } void CalculateReport(void) if((getupper()==120) && (getlower()==80)) cout<<"Blood pressure is normal..."; else cout<<"blood pressure is not normal.."; }; int main(void) { Patient P; system("pause"); return 0; }

Share this post


Link to post
Share on other sites
Advertisement
Using standard C++, you should include <iostream> and <cstdlib> instead of iostream.h and stdlib.h. Also, it's a bad idea to begin variables with an underscore because they are reserved for the implementation. Also, you never seem to set the name in your Patient constructor.

Those probably won't remove errors though; what errors are you getting?

Share this post


Link to post
Share on other sites
Quote:
Original post by Nitage
You're missing an opening brace for your CalculateReport() function.

And a closing brace.
And use BP.getupper() instead of getupper()
And use BP.getlower() instead of getlower()

And that's just the errors...

Share this post


Link to post
Share on other sites
Some additional syntax, style, and design correctness issues of note:


  • Use std::string for strings in C++.

  • Don't use system("pause") to "prevent the window from closing," run the program in the proper fashion from the IDE or the command prompt instead.
  • Putting your program flow in your constructor like that is horrible design.

  • Enforce invariants in your class methods. Presumably Blood_Pressure::upper should always be greater than Blood_Pressure::lower.



Also, format your code on these forums with [ source ] and [ /source ] tags (minus the extra spaces).

And in the future, post the error message(s) when asking for help and do something to indicate that you've actually tried to solve the problem yourself. You'll get more helpful responses that way.

Share this post


Link to post
Share on other sites
Changes marked with CHANGE comments, things which should be changed marked with NOTE comment.


// CHANGE:
// iostream is the correct header
// conio.h doesn't exist in C+
// cstdlib is the correct header, but it isn't used so we remove it.
// string added so we can use strings
#include <iostream>
#include <string>

class Blood_Pressure{
private:
float upper;
float lower;

public:
Blood_Pressure()
{
// NOTE:
// 0.0 is a double, replaced with 0.0F which is a float, not an error, but bad programming style
// NOTE:
// Initialization lists is better for initializing variables
upper=lower=0.0;
}
~Blood_Pressure()
{
}
// NOTE: Some names starting with _ are reserved for implementation
// NOTE: When passing by value you shouldn't make the value const, there is no reason why you would.
void setupper(const float _upper)
{
upper=_upper;
}
// NOTE: Some names starting with _ are reserved for implementation
// NOTE: When passing by value you shouldn't make the value const, there is no reason why you would.
void setlower(const float _lower)
{
lower=_lower;
}
// CHANGED: void to indicate 0 paramters is a C habit, not C++
// CHANGED: This function has been made const, because it doesn't change the state of the object.
float getupper()const
{
return upper;
}
// CHANGED: void to indicate 0 paramters is a C habit, not C++
// CHANGED: This function has been made const, because it doesn't change the state of the object.
float getlower()const
{
return lower;
}
};

class Patient{
private:
// CHANGED:
// Using a real string instead of a finite sequence of bytes
std::string name;
// NOTE: Inconsistent naming convention, before you used lowercase for member variables.
Blood_Pressure BP;
public:
Patient()
{
// CHANGE:
// Changed to a real string.
std::string name;
// Change it is NOT cout and cin, it's std::cout and std::cin.
std::cout <<"enter your name..";
std::cin>>name;
// NOTE: Some names starting with _ are reserved for implementation
float _upper,_lower;
std::cout<<"enter upper limit..";
std::cin>>_upper;
BP.setupper(_upper);
std::cout<<"enter lower limit..";
std::cin>>_lower;
BP.setlower(_lower);
CalculateReport();
}
~Patient()
{
}

// NOTE: Some names starting with _ are reserved for implementation
// CHANGE: Changed to a real string. Passed by const-reference.
void setname(const std::string& _name)
{
// CHANGE: using different function to copy a real string.
// NOTE: When using the C library you should always make sure to use the bounded versions of the functions. Then you could tell the compiler to not copy more than 20 bytes. One of the many reasons we use real strings.
// NOTE: You should use std::strcpy instead of strcpy
name=_name;
}
// CHANGED: void to indicate 0 parameters is a C habit, not C++
// CHANGED: real string as return value.
// CHANGED: This function has been made const, because it doesn't change the state of the object.
// NOTE: That function was a potential bug anyway, you can't just return a pointe like that
// you need to make a copy of that and return that and then make it the caller's responsibility
// to delete it. So if someone had got your string deleted your patient, then the string they got
// would be invalid because the patient destroyed it.
// One of the many reasons we use real strings.
std::string getname() const
{
return name;
}

// CHANGED: void to indicate 0 paramters is a C habit, not C++
// CHANGED: This function has been made const, because it doesn't change the state of the object.
void CalculateReport()const
// CHANGED: { missing, added it.
{

// NOTE: Don't you consider 120.00000001 and 80.0000001 normal? Also you can't just compare floating point numbers like that, you need to accept all numbers within a certain range (because 120 and 80 might not be representable by your floating point format. For example if( getupper()-120 < 0.001 ) would accept all values which aren't farther than 0.001 away.
if((getupper()==120) && (getlower()==80))

cout<<"Blood pressure is normal...";

else

cout<<"blood pressure is not normal..";
// CHANGED: } missing, added it. Functions ALWAYS need brackets.
}

};


// CHANGED: void to indicate 0 parameters is a C habit, not C++
int main()
{
Patient P;
// CHANGED: system("pause") aren't guaranteed to work, use while(std::cin.get()); instead.
while(std::cin.get() );
return 0;
}


Share this post


Link to post
Share on other sites
Sounds like homework to me.

BTW, why should one not use system("pause") at the end of a console program like this?

Share this post


Link to post
Share on other sites
Quote:
Original post by davidx9
Sounds like homework to me.

BTW, why should one not use system("pause") at the end of a console program like this?


Well, the real name is std::system so the statement isn't syntactically correct (unless a certain deprecated header is included, but in a C++ program this header should never be included). Also the behavior of this is platform-dependent. It basically invokes the command interpreter with "pause" this will do something unpredictable, or it might do nothing. In most cases it'll do nothing, Windows being one of the exceptions where it works, but that doesn't mean we should tie ourself to this one platform when better alternatives exist.

Share this post


Link to post
Share on other sites
Quote:
Original post by CTar
Changes marked with CHANGE comments, things which should be changed marked with NOTE comment.

// NOTE: When passing by value you shouldn't make the value const, there is no reason why you would.
void setupper(const float _upper)


I dont want to sound picky but you should use const even when passing by value IF it is SEMANTICALLY correct, also the other reason you would which doesnt really matter as much is that the compiler is able to perform additional optimizations if you do :-), or at least i think so it was in another thread awhile ago so please correct me if im wrong.

Edit: found the thread if anyones interested http://www.gamedev.net/community/forums/topic.asp?topic_id=416869, its the last post

Share this post


Link to post
Share on other sites

Quote:

// CHANGED: void to indicate 0 parameters is a C habit, not C++


That's in the Coding Standards document where I work :|

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!