0

I asked this question here (thinking I would help people) Creating an unnecessary getter and unearthed a huge area of ignorance I have.

In this answer it was pointed out to me I had a fatal flaw in my code and I quote for ease:

"This is wrong:

public Patient(final String ptNo, final String ptName,
        final String procDate, final int procType, final String injury,
        final String drName) throws IOException
{
    Patient.ptNo = getPtNo();
    Patient.ptName = getPtName();
    Patient.procDate = getProcDate();
    Patient.procType = getProcType();
    Patient.injury = getPtNotes();
    Patient.drName = getDrName();
}

As you're completely ignoring all values passed in as parameters. Instead do:

public Patient(final String ptNo, final String ptName,
        final String procDate, final int procType, final String injury,
        final String drName) throws IOException
{
    Patient.ptNo = ptNo;
    Patient.ptName = ptName;
    Patient.procDate = procDate;
    Patient.procType = procType;
    Patient.injury = injury;
    Patient.drName = drName;
}

Where here you're setting your class's fields with the parameter values."

What I don't understand, is why the values are being ignored. I call separate methods for eg:

public static String getPtName()
{
    System.out.print("Enter patient name: \n");
    try
    {
        ptName = stdin.readLine();
    } catch (IOException e)
    {
        System.out.println("Error! Enter a valid option.");
        getPtName();
    }
    return ptName;
}

So I thought that was the same, in a longer way of writing the second block of code.

Can someone please explain to me, why it is different?

edit The assignment requirements from uni.

C) Provide a constructor for the class that accepts the patient number (a String), patient name (a String), procedure date (a String in the format dd/mm/yy), procedure type (an int), the injury description (a String) and the doctor name of the physician who is administering the patient’s treatment.

This constructor should initialise the instance variables with the corresponding parameter values that have been passed in - it should also initialise the patient notes instance variable to the injury description that was passed in initially and initialise the patient status instance variable to ‘S’ (indicating that the new patient has had a procedure Scheduled).

public Patient (String patientNo, String patientName, 
                 String procedureDate, int procedureType,
                 String injuryDescription, String doctorName)

D) Implement accessors for the patient number, patient name, procedure date, patient notes and doctor name instance variables.

Community
  • 1
  • 1
  • 1
    Wow, by "convention" a getter returns de `ptName` not ask by input keyboard, that's the point. 2nd point you are setting properties to static variables in that way in your constructor, not even in your instance that create! – nachokk Nov 04 '13 at 00:00
  • You seem to have many profound gaps in your understanding of OOP, which are beyond the scope of this forum to address. Find some online tutorials and read, read, read. – Bohemian Nov 04 '13 at 00:01
  • None of your fields should be static. In fact the only thing that should be static is your main method, that's it. You shouldn't have any user input in your Patient class. That should be in a user interface class, or perhaps in your situation the main method. Read a chapter on getters, setters, constuctors and such. You need to study to get all this down. – Hovercraft Full Of Eels Nov 04 '13 at 00:16
  • 2
    If you have requirements for the assignment, post them, but I will repeat in a different way: the Patient class should be for holding and information that pertains to a *single* patient, hence all that data should not be static. The patient class should have classic getters and setters, methods that allow you to set the patient's fields and to find out what the fields hold, but the actual user interaction, the JOptionPanes, the Scanners, the prompts, have no business in the Patient class. – Hovercraft Full Of Eels Nov 04 '13 at 00:21
  • 1
    You state that `"... eclipse throws up errors and that is why it has developed to be static, but that could be because there's something wrong with my code elsewhere ..."` -- then you're fixing things backwards. The solution is not to make things static but to never call them in a static way. That's why all your Patient objects hold the same data. Again, **nothing** should be static other than the main method (with a few exceptions). – Hovercraft Full Of Eels Nov 04 '13 at 00:22
  • 1
    Your question really parses for us that you need to do a lot more studying on your own. We are good for answering specific questions, but you yourself have to study to gain a foundation of the basic concepts. We really are not in a position to teach. You should be spending your time now concentrating on this more and less on stackoverflow. – Hovercraft Full Of Eels Nov 04 '13 at 00:23
  • 1
    @Skippy: as I suspected, your assignment states nothing about having user input in the Patient class, and in fact you should have none, no JOptionPanes, nothing. And certainly no static methods or fields. Please see edit to my answer in your previous question for a simple example of exactly what they're requesting here. Note that all UI goes in a class separate from Patient, or in my example, from Client. – Hovercraft Full Of Eels Nov 04 '13 at 00:41

2 Answers2

3

In the first example:

public Patient(final String ptNo, final String ptName,
               final String procDate, final int procType,
               final String injury, final String drName)
               throws IOException

The constructor never uses the variables that are passed in. That is, they don't appear anywhere within the body of the function.

It's particularly confusing because the parameters to the function have exactly the same names as the class variables, but Patient.ptNo is not the same variable as the parameter ptNo. (Actually, Patient.ptNo should be this.ptNo, because it belongs to this particular instance of the class. Patient.ptNo would refer to a single value that's common to all objects of type Patient.) When you write the corrected form

this.ptNo = ptNo;

you're setting the class variable to the same value as the parameter variable. In fact, in this case, you must qualify the class variables, otherwise the parameter variables will be used instead.

Some programmers follow conventions, such as adding a prefix or suffix to the names of all class variables, to make it impossible for these name collisions to occur. So, for example, you might have

this.c_ptNo = ptNo;

or just

c_ptNo = ptNo;

where the c_ prefix indicates a class variable.

Adam Liss
  • 47,594
  • 12
  • 108
  • 150
  • Adding to your answer it's not a good approach use `static` variable in this case. Has no sense at all. – nachokk Nov 04 '13 at 00:02
  • Sorry; I don't follow you. Which variable would be `static`? – Adam Liss Nov 04 '13 at 00:07
  • 1
    Don't take the comments personally. Too many programmers think they need to prove how smart they are, and they forget the questions and mistakes we all made when we were learning. (I've been programming since the Apple ][ first came out, I'm older than you, and I _still_ make silly mistakes ... but I'm no longer ashamed to ask "dumb" questions.) Keep asking questions, and keep learning. – Adam Liss Nov 04 '13 at 00:31
2
    public Patient(final String ptNo, final String ptName,
                   final String procDate, final int procType, final String injury,
                   final String drName) throws IOException

You are ignoring all of the values passed into the constructor. If you want the user to manually enter the values the way you have it can be done with a parameterless constructor.

public Patient() {
    /*your code*/
}

You original code requires all that information to be passed in when the Patient object is created.

That being said, you shouldn't be accepting user input in a constructor call. You should just make standard getters and setters to deal with those fields.

Patient.<whatever> should just be this.<whatever>

user1231232141214124
  • 1,339
  • 3
  • 16
  • 22