Sign in to follow this  
MubasharIqbal

plz correct the code

Recommended Posts

MubasharIqbal    100
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
Simian Man    1022
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
Wan    1366
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
jpetrie    13098
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
CTar    1134
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
CTar    1134
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
Julian90    736
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
zdlr    266

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
jpetrie    13098
Quote:

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

Because it's Flat Out Stupid. It is equivalent to things like "We don't call delete on this particular pointer, because the OS will clean up the memory when our process dies" and "the user has selected the quit option, so let's quit real fast: int *p = 0; *p = 10;."

And so on. Yes, they achieve the desired end result, but they do it in obviously stupid and potentially dangerous, non-portable ways. Note that non-portability is generally the least of the problems here.

When you run a program built for a console environment from your IDE, the IDE spawns a console environment. The program runs to termination and quits. At this point, most console environments will terminate themselves because they are smart enough to know that they were launched with the express purpose of running a single command (your executable). This is the correct, desired behavior of both the console environment and your program.

A program built for a console environment is designed to run in a console environment. Artificially pausing the program prior to exit is counter-intuitive in that environment. Think of how annoying it would be use to the command program if every time your listed a directory, changed directories, et cetera, you were promted to "press a key to continue." Think of how impossible it would be to write a batch file uses a program that requires user input to fully terminate itself, even if it otherwise requires no user interaction. It's exactly the same as the obnoxious "You picked the quit option, are you sure you want to quit?" prompts (these are forgivable only if quitting would cause you to lose state or information, such as when you have unsaved work; otherwise they are redundant and counter-intuitive). Just don't do it.

Any decent IDE (including Visual Studio -- it's either Run or Run Without Debugging, I tend to forget which, my keymapping is nonstandard, and I don't have VS installed on the machine I'm currently at) has a means to invoke a console-environment program and persist the console window. This could be as simple as generating a batch file that calls your program, and then calls "pause" or some other platform-specific delay mechanism. The difference, however, is that that method does not hard-code incorrect delay logic into your program.

Share this post


Link to post
Share on other sites
CTar    1134
Quote:
Original post by Julian90
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.


That's the point, the optimizations you talk about comes from the fact that the code have more knowledge about the function (it doesn't alter the value). When passing by value you get no additional information, so the function is exactly the some seen from the outside.

The only change is that you will not be able to reuse the parameter inside the function (leading to slower and less intuitive code).

Almost all other kind of consts should be used however because they DO change the function's contract.

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

Well it could be in there for many reasons.

You might have migrated from C and kept some of the still legal standards to keep the code consistent. This is such a small issue that there is no reason to really stop doing it, however neither are there any reasons to start doing it.

Also many C++ programmers (still the minority, but a significant part) are still programming in C, but using a couple of C++ features.

So do you believe it's a good thing that you're forced to do it? Can you tell us why such a thing is required? If you present good points then some of us might see your point and start doing it ourselves.

EDIT: Also I second what jpetrie said about explicitely pausing, however I often feel beginners' have others just as big design problems.

Share this post


Link to post
Share on other sites
zdlr    266
I was merely casually observing the fact. No insinuation that it's either good or bad. The extra eight characters typed per parameter-less function doesn't really impact on my day too much :P

The surprised face was that I had never questioned the validity of the standard, and just blindly accepted it! I think you're probably right that at one time the company used C as its main language.

Share this post


Link to post
Share on other sites
davidx9    184
Quote:
Original post by jpetrie
Quote:

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

Because it's Flat Out Stupid. It is equivalent to things...


I think when you're just starting out, and can barely manage to construct a class, portability and efficient batch execution is not high on the list of concerns; nor is the desire to customize your build environment.

Of course jpetrie, you are absolutely technically correct, but quick fixes such as system("pause") could in theory accelerate the rate of learning new techniques, as long as you're careful later on.

Share this post


Link to post
Share on other sites
jpetrie    13098
I disagree. "Control-F5" verus "F5" (or whatever the appropriate key combination swappage is) to persist the console window after execution is trivial to learn (just as trivial as system("pause")) and get into the habit of. It requires no customization of the environment in any environment I've every worked in.

I see no reason that system("pause") should ever be used or taught to beginners in this respect; there's little point in learning and using things you know to be incorrect with the rationale that "you'll change it later."

Share this post


Link to post
Share on other sites
Guest Anonymous Poster   
Guest Anonymous Poster
Quote:
Original post by jpetrie
I disagree. "Control-F5" verus "F5" (or whatever the appropriate key combination swappage is) to persist the console window after execution is trivial to learn (just as trivial as system("pause")) and get into the habit of. It requires no customization of the environment in any environment I've every worked in.
How about Dev-C++? I don't think it has such a feature. Even the example console app it provides has system("pause") in it IIRC. And I see no reason not to use it during development if there's no better alternative.

Share this post


Link to post
Share on other sites
Oluseyi    2103
Quote:
Original post by davidx9
I think when you're just starting out, and can barely manage to construct a class, portability and efficient batch execution is not high on the list of concerns; nor is the desire to customize your build environment.

Of course jpetrie, you are absolutely technically correct, but quick fixes such as system("pause") could in theory accelerate the rate of learning new techniques, as long as you're careful later on.

Unlearning bad habits can be harder than learning the correct habits in the first place. Plus, if the objective is to accelerate learning, perhaps a better solution is to choose a language other than C++?

Quote:
Original post by Anonymous Poster
How about Dev-C++? I don't think it has such a feature. Even the example console app it provides has system("pause") in it IIRC. And I see no reason not to use it during development if there's no better alternative.

But there are. And the better alternatives are free, too.

Dev-C++ isn't being actively developed. There's no reason to attach yourself to a dead product.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster   
Guest Anonymous Poster
Quote:
Original post by Oluseyi
But there are. And the better alternatives are free, too.
Such as? Also consider how light-weight, easy to install and small Dev-C++ is. Even though I have VC++ installed, I sometimes use Dev-C++ for small tests because it starts up faster, requires less memory and uses GCC as a compiler (in case I need to check differences between VC and GCC). And I see no problem using it for such purposes. system("pause") doesn't bother me enough to even consider other options.

Share this post


Link to post
Share on other sites
Zahlman    1682
Just because I'm me...

#include <iostream>
#include <string>

using namespace std;

struct Blood_Pressure {
float upper;
float lower;

Blood_Pressure(float upper, float lower) : upper(upper), lower(lower) {}
};

class Patient {
std::string name;
Blood_Pressure BP;

public:
Patient(const std::string& name, const Blood_Pressure& BP) :
name(name), BP(BP) {}

bool bloodPressureIsNormal() {
return BP.upper == 120 && BP.lower == 80;
}
};

int main() {
std::string name;
cout << "Enter your name: ";
std::getline(cin, name);

float upper, lower;
cout << "Enter upper limit: ";
cin >> upper;
cout << "Enter lower limit: ";
cin >> lower;

Patient p(name, Blood_Pressure(upper, lower));

if (p.bloodPressureIsNormal()) {
cout << "Blood pressure is normal." << endl;
} else {
cout << "Blood pressure is not normal." << endl;
}
}


Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this