Re: Beg Class challenge for a returning student
From: Thomas Matthews (Thomas_MatthewsSpitsOnSpamBots_at_sbcglobal.net)
Date: 08/30/04
- Next message: Rensjuh: "Tutorial for beginner/ Tutorial voor beginner"
- Previous message: Kay: "Re: getline and linked list problem"
- In reply to: mchoya: "Re: Beg Class challenge for a returning student"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Date: Mon, 30 Aug 2004 15:04:08 GMT
Thanks.
top-post.
don't
Please
mchoya wrote:
[Re-arranged]
> "mchoya" <mickifox@sbcglobal.net> wrote in message
> news:ahoYc.41$9g6.3@newssvr16.news.prodigy.com...
>
[snip]
>>This executable is supposed to 1. create an array of Patient objects, then
>>2. present a user menu to a. read Patient data from a user specified file,
>>b. display data. c. quit program.
>>
>>Facts: max patients = 1000.
>>
>>Requirements: cannot change header or implement file; use an array; handle
>>user errors
>>
>>So here it is, any assistance would be extremely appreciated.
>>
>>HEADER
>>#ifndef PATIENT_H
>>#define PATIENT_H
>>
>>#include <iostream>
>>#include <string>
>>
>>using namespace std;
>>
>>class Patient
>>{
>>public:
>> Patient();
>> Patient(string fName, string lName, string idString, int bYear);
>>
>> string getName() const;
>> int getAge() const;
>> string getID() const;
int bYear() const;
>>
>> void setFirstName (string fname);
Use const reference to avoid the overhead of passing a whole
string structure:
void setFirstName(const string& first_name);
>> void setLastName (string lname);
See above.
>> void setID(string idString);
See above.
>> void setBirthYear(int bYear);
>>private:
My preference is to include some whitespace between
the public, private and protected sections.
>> string firstName;
>> string lastName;
>> string ID;
>> int birthYear;
You _may_ want this as an unsigned signed int since
people can't be born in a negative year.
>>};
>>#endif
>>
>>HEADER DEFINITIONS
>>#include <iostream>
>>#include <string>
>>#include "patient.h"
>>
>>using namespace std;
>>
>>Patient::Patient() {
>> setFirstName("First");
>> setLastName("Last");
>> setID("Z9999");
>> setBirthYear(9999);
>>}
You may want to leave the fields empty for the default constructor.
>>Patient::Patient(string fName, string lName, string idString, int bYear) {
>> setFirstName(fName);
>> setLastName(lName);
>> setID(idString);
>> setBirthYear(bYear);
>>}
Prefer to use initialization lists:
Patient::Patient(const string& fName,
const string& lName,
const string& idString,
unsigned int bYear)
/* here is the initialization list */
: firstName(fname),
lastName(lname),
ID(idString),
birthYear(bYear)
{
}
If initialization is more complicated, then use an
initialization method and have each constructor call
it.
>>string Patient::getName() const {
>> return lastName + ", " + firstName;
>>}
You may want to change the title of this method.
Some users may want to have both last name first
and first then last layouts.
>>int Patient::getAge() const {
>> return birthYear;
>>}
>>string Patient::getID() const {
>> return ID;
>>}
>>void Patient::setFirstName(string fName) {
>> firstName = fName;
>>}
>>void Patient::setLastName(string lName) {
>> lastName = lName;
>>}
>>void Patient::setID(string idString) {
>> int i = idString.size();
>> if (i == 5) {
Where did this magic number come from?
>> ID = idString;
>> }
>> else {
>> cerr <<"\nIllegal Patient ID: " <<idString<<" using Z9999"<<endl;
Explain to the user why the ID is illegal:
cerr << "Patient ID string too long.".
Although you may want to through an exception because
this method assumes that cerr will be available and
the user will be looking at it. With an exception,
the function using Patient::setID will get to choose
what to do with this error.
>> ID = "Z9999";
>> }
>>}
>>void Patient::setBirthYear(int Year) {
>> if ((Year >1873) && (Year < 2003)) {
>> birthYear = 2003 - Year;
>> }
>> else {
>> cerr<<"\nIllegal Year: "<<Year<<" using 9999"<<endl;
>> birthYear = 9999;
>> }
>>}
Why bother altering the year. There is nothing to
gain, but much more to lose. Just check for boundary
conditions and throw an exception. An integer, signed
or unsigned, has enough capacity for a persons birth
year.
>>
>>**** Problem Driver ***********
Hmmm, does the driver have a problem? :-)
>>#include <iostream>
>>#include <string>
>>#include <fstream>
>>#include <iomanip>
>>#include "patient.h"
>>
>>using namespace std;
>>
>>char resp;
>>void GetList();
>>bool getPatient(ifstream &fin, Patient &P);
>>const int MaxPatientSize = 1000;
>>
>>int main()
>>{
>>
>> Patient P[MaxPatientSize];
>> GetList();
>>
>> do{
>> cout <<"\nWould you like to read another Patient File? (y or n):";
>> cin >> resp;
>> }while ((resp != 'y') && (resp != 'Y') && (resp != 'n') && (resp !=
>>
>> 'N'));
Do yourself a favor and research the std::toupper and std::tolower
methods. This will simplify to:
cin >> resp;
resp = std::toupper(resp);
} while (resp != 'Y' && resp != 'N');
>>
>> if ((resp == 'y') || (resp == 'Y'))
if (resp == 'Y') /* see std::toupper */
>> GetList();
>> else
>> cout <<"Good Bye!"<< endl;
>>
>> return 0;
See EXIT_SUCCESS in <cstdlib>
return EXIT_SUCCESS;
>>}
>> void GetList()
>> {
>> cout<<"Enter name of Patient List to be read: ";
>> cin.getline(
This line is an error. The compiler should have said something.
>> ifstream fin(file_name.c_str());
>> if (!fin)
>> {
>> cerr<<"\nCannot open Patient List. Good Bye\n";
>> exit(1);
See EXIT_FAILURE in <cstdlib>
exit(EXIT_FAILURE);
This is bad karma to have a function terminate the program.
The function should throw an exception or return a status.
Let the main() function be the only function that terminates
the program.
>> }
>> do
>> {
>> cout<<"Would you like to display your Patient List? (y or n): ";
>> cin >> resp;
>> }while ((resp != 'y') && (resp != 'Y') && (resp != 'n') && (resp !=
>>'N'));
>> if ((resp == 'y') || (resp == 'Y'))
std::toupper or std::tolower. 'Nuff said.
>> {
>> for (P = Patient(),
>> while(getPatient(fin, P))
What language is this?
1. Don't mix while and for loops.
2. A "for" loop implies one or more iterations.
3. A "while" loop implies zero or more iterations.
More bad karma.
>> {
>> cout<<"\n\n\n"<<setw(25) <<setiosflags(ios::left)<<"Patient Name"
>><<setw(20)
>> <<setiosflags(ios::left)<<"Patient ID" <<setw(15)
>> <<setiosflags(ios::left)<<"Patient Age"<<endl;
>> cout <<setw(25)<<setiosflags(ios::left)<<P.getName() <<setw(20)
>> <<setiosflags(ios::left)<<P.getID() <<setw(15)
>> <<setiosflags(ios::left)<<P.getAge()<<"\n\n";
Looks like your Patient class would really benefit by overloading
operator <<. This would simplify to:
cout << P;
>>
>> fin.close();
So, why is the file closed after reading one patient?
>> P = Patient();
This isn't necessary. No reason to clear an object when
it will be overwritten; just a waste of time and energy.
Poor karma.
>> }
>> else
>> return;
>> }
>>bool getPatient(ifstream &fin, Patient &P)
>>{
Note: This function could be replaced by overloading
the operator>> in the Patient class.
>> bool isOK = true;
>> string name = "";
>>
>> if (getline(fin, name, '\n') && isOK)
The first line does not need to check the isOK
variable.
Beware of using cut and paste.
>> fam.setFirstName(name);
>> else
>> isOK = false;
>>
>> if (getline(fin, name, '\n') && isOK)
>> fam.setLastName(name);
>> else
>> isOK = false;
>>
>> if (getline(fin, name, '\n') && isOK)
>> fam.setID(name);
>> else
>> isOK = false;
>>
>>/*f (getline(fin, name, '\n') && isOK)
>> fam.setBirthYear(name);
>> else
>> isOK = false; */
>>
>> return isOK;
>>}
My suggestion is to implement two methods in your
Patient class that load and store to a string.
For example, your above input function will fail
if the data fields are all on one line separated
by tabs or commas. You will need another method
when retrieving the data from a database or
other input source.
My experience is to:
1. Input the data to a string.
2. Have the object initialize its members from
the given string.
In your program, the above simplifies to:
std::string text_line;
std::string patient_record;
std::vector<Patient> patient_list;
while (fin)
{
for (unsigned int i = 0;
(i < LINES_PER_RECORD) && (getline(fin, text_line);
++i;
{
if (i > 0)
{
patient_record += '\t';
}
patient_record += text_line;
}
if (i < LINES_PER_RECORD)
{
// Handle case of not enough data.
}
else
{
Patient new_patient;
new_patient.load_from(text_line);
patient_list.push_back(new_patient);
}
}
>
>
>
> Of course your right, sorry.
>
> Well, I'm receiveing "unresolved external links" to the header
> getAge, getID, getName.
>
> But , I guess I'm looking for direction in the following, regarding the
> executable file:
> can I declare an array while initializing an object; ex: Patient
> P[MaxSize], where MaxSize has been declared.(my goal being to limit the
> number of Patient files (think hospital beds) to MaxSize
> have I chosen appropriate parameters for the function getList in the
> executable file
>
> Thanks
>
You also have to make sure that your linker is fed all object
modules or your project includes all your source files.
--
Thomas Matthews
C++ newsgroup welcome message:
http://www.slack.net/~shiva/welcome.txt
C++ Faq: http://www.parashift.com/c++-faq-lite
C Faq: http://www.eskimo.com/~scs/c-faq/top.html
alt.comp.lang.learn.c-c++ faq:
http://www.comeaucomputing.com/learn/faq/
Other sites:
http://www.josuttis.com -- C++ STL Library book
- Next message: Rensjuh: "Tutorial for beginner/ Tutorial voor beginner"
- Previous message: Kay: "Re: getline and linked list problem"
- In reply to: mchoya: "Re: Beg Class challenge for a returning student"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Relevant Pages
|
