My procedural mind needs a dose of OO

[Chuck Beauregard]

I realize that there are all kinds of ways to solve a problem. While I still have difficultly reading some of the Java examples, they appear to be more elegant than my code. Hence I think I'm stuck in a procedural mindset trying to do OO programming.

I've written several modules to solve the following problem, but elegant it is not. As a beginning Java forum, perhaps I could get some pointers on how to "right the ship".

The problem is this:

Users will need to enter their current membership roster with handicap data to start the program. This data is available from an independent (very independent) source called GHIN. GHIN has standard reports for each club, which are not very standard at all, and are NOT modifiable. These reports can be exported in a number of ways, Excel, Text, PDF to name a few.

The data desired from this report is "Name" , "H.I" and potentially "GHIN #". However all that is mandatory is "Name".

The issue is the report has a header of varying sizes, it could be 1 line with only "Name", "H.I." and "GHIN" or a subset of them, OR the header could be several lines long. I seen examples with 8 lines of header and footer. These headers appear on each page of the report. In addition the space between the informational column is random. To be effective my program must handle all of them seamlessly.

While this program appears to separate the data properly it doesn't build the instances of GCMembers or a List of the members.

If this is an inappropriate use of the forum, I apologize.

package viewOfGolfSystem;

import java.io.BufferedReader;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;

import javax.swing.JFileChooser;
import javax.swing.filechooser.FileNameExtensionFilter;

public class ReadTextFile {
	// //////////////// getFilePath Method //////////////////////////////
	/*
	 * While this FileChooser works it doesn't respond to double clicks well in
	 * fact I'd categorize it as sub par
	 */
	static String getFilePath() {
		JFileChooser chooser = new JFileChooser();
		FileNameExtensionFilter filter = new FileNameExtensionFilter(
				"TXT and CSV files", "txt", "csv");
		chooser.setFileFilter(filter);
		int returnVal = chooser.showOpenDialog(chooser);
		if (returnVal == JFileChooser.APPROVE_OPTION) {
			chooser.addChoosableFileFilter(filter);
			chooser.showOpenDialog(null);
		} else {
			System.out.println("No File choosen, try again");

		}
		File f = chooser.getSelectedFile();

		return f.getAbsolutePath();

	}

	// ////////// Main Method ////////////////////////////////////////
	public static void main(String[] args) {
		String line;
		boolean headerFound = false;
		GetHeader GH = new GetHeader();
		File file = new File(getFilePath()); // uses method to get file path

		try (BufferedReader br = new BufferedReader(new FileReader(file))) {
			int count = 0;
			while ((line = br.readLine()) != null) {
				String[] m = line.split("\t");
				/*
				 * This is to read a club's membership roster from a GHIN
				 * report. The GHIN report can come out in a large number of
				 * formats beyond the users control so this input file must be
				 * flexible. The standard in these reports ia a "header line".
				 * This line as supplied by GHIN and has "name", "Ghin #" and
				 * "H.I." columns of interest. GHIN # is not commonly there. In
				 * fact this program should work if there is only a "name"
				 * column, the other two are optional.
				 */

				if (!GH.getHeader(m)) {

					if (m.length > 1 && m[0] != null && !m[0].isEmpty()) {
						GCMembers players;
						if (GH.getIn() > -1 && GH.getIh() > -1
								&& GH.getIg() > -1)
							players = new GCMembers(count, m[GH.getIn()],
									m[GH.getIh()], m[GH.getIg()]);
						else if (GH.getIn() > -1 && GH.getIh() > -1)
							players = new GCMembers(count, m[GH.getIn()],
									m[GH.getIh()]);
						else
							players = new GCMembers(count, m[GH.getIn()]);
					}
					count++;
					System.out.println("\n" + count + " : " + line);
					System.out.print("this is m :");
					for (int ix = 0; ix < m.length; ix++) {
						System.out.print(" " + m[ix]);
					}
				} else { // get header locations for data transfer phase
					headerFound = true;

				}
			}

			if (!headerFound) {
				System.out.println("Error 0002:  "
				   + "The membership file you choose "
				   + "did not have a header, try again.");
			}

		} catch (FileNotFoundException e) {
			System.out.println("File not found " + file.toString());

		} catch (IOException e) {
			System.out.println("Unable to read file " + file.toString());
		}
	}
}
package viewOfGolfSystem;

public class GetHeader {

	// Boolean iReturn = false;
	private int in = -1;
	private int ig = -1;
	private int ih = -1;

	public int getIn() {
		return in;
	}

	public void setIn(int in) {
		this.in = in;
	}

	public int getIg() {
		return ig;
	}

	public void setIg(int ig) {
		this.ig = ig;
	}

	public int getIh() {
		return ih;
	}

	public void setIh(int ih) {
		this.ih = ih;
	}

	public boolean getHeader(String[] m) {

		boolean iReturn = false;

		int k = Math.min(10, m.length);

		for (int i = 0; i < k; i++) {
			String mTrim = m[i].trim();
			if (mTrim.equalsIgnoreCase("name")) {
				setIn(i);
				iReturn = true;
			}
			if (mTrim.equalsIgnoreCase("GHIN #")) {
				setIg(i);
			}
			if (mTrim.equalsIgnoreCase("h.i.")) {
				setIh(i);
			}
		}
		System.out.println("\n in : " + getIn() + " ig : " + getIg() + " ih : "
				+ getIh() + "  " + iReturn);
		return iReturn;
	}
}
package viewOfGolfSystem;

public class GCMembers {
	int ID;
	String name;
	String ghin;
	String hIndex;
	String eMail;

	public GCMembers(int iD, String name) {

		ID = iD;
		this.name = name.replace("\"", "");
		System.out.println("\nID : " + ID + "\n Name : " + this.name);
	}

	public GCMembers(int iD, String name, String hIndex) {

		ID = iD;
		this.name = name.replace("\"", "");
		this.hIndex = hIndex;
		System.out.println("\nID : " + ID + "\n Name : " + this.name
				+ "\n HI : " + hIndex);
	}

	public GCMembers(int pID, String pName, String pHIndex, String pGhin,
			String pEMail) {
		name = pName.replace("\"", "");
		ghin = pGhin;
		hIndex = pHIndex;
		eMail = pEMail;
		this.ID = pID;

		System.out.println("\nID : " + ID + "\n Name : " + name + "\n GHIN : "
				+ ghin + "\n HI : " + hIndex + "\n eMail : " + eMail);
	}

	public GCMembers(int pID, String pName, String pHIndex, String pGhin) {
		name = pName.replace("\"", "");
		ghin = pGhin;
		hIndex = pHIndex;
		this.ID = pID;
		System.out.println("\nID : " + ID + "\n Name : " + name + "\n GHIN : "
				+ ghin + "\n HI : " + hIndex);
	}
}

[Bear Bibeault]

Heh. I didn't make it past line 12. The very fact that you have a class named ReadTextFile is a problem. Classes model things, not behaviors.

Etch-a-sketch everything and grab a pencil and paper and write down what you are trying to accomplish. Underline the nouns. These are the candidates for modeling via classes, not the verbs.

Does that change your approach?

[Chuck Beauregard]

Bear Bibeault wrote:Heh. I didn't make it past line 12. The very fact that you have a class named ReadTextFile is a problem. Classes model things, not behaviors.

Etch-a-sketch everything and grab a pencil and paper and write down what you are trying to accomplish. Underline the nouns. These are the candidates for modeling via classes, not the verbs.

Does that change your approach?

Answer maybe.......

So if I said: Take a file of the membership, read it eliminating all headers and footers, and build a membership roster.
. The file might be an Excel file or a tab delimited text file. Data will be spread out over the first 10 columns and will have to be found using the header. Only the "name" is absolutely necessary for the membership file, but the addition of "H.I.", "Ghin #" and "Email address" would make the membership file more complete.

Why maybe, well if "methods" are supposed to be the verbs, than "read", "build", "might be" = "convert", and "found" would fit the bill.

The Classes would be "File", "roster" and "header". These are in essence what I had, a Textfile Class with a method to get the path, a "main" method to read the file assisted by a method (called GetHeader Class) to isolate the header. I'm feeling like my bites are too big and need to be more mouse than alligator size, as these changes seem like semantics more than anything.

Now that said, no matter what size the bite is, areas like this just don't seem to me to be good code.

		for (int i = 0; i < k; i++) {
			String mTrim = m[i].trim();
			if (mTrim.equalsIgnoreCase("name")) {
				setIn(i);
				iReturn = true;
			}
			if (mTrim.equalsIgnoreCase("GHIN #")) {
				setIg(i);
			}
			if (mTrim.equalsIgnoreCase("h.i.")) {
				setIh(i);
			}
		}

I want to figure out how to effectively use Lists to compare the contents verses a loop, yet still know which header item is in which column.

[Bear Bibeault]

mTrim? setIn? setIg? setIh?

These names are meaningless.

Another tip: meaningful names help to make code understandable.

It's hard to recommend a good way to approach something when what that something is trying to do is masked by poor names that obscure the meaning.

Consider the difference in readability between:

boolean sdsd = asdsda.ahjgsjdgsja();

and

boolean canTeleport = person.isWearingARedHat();

[Chuck Beauregard]

Bear Bibeault wrote:mTrim? setIn? setIg? setIh?

These names are meaningless.

Another tip: meaningful names help to make code understandable.

It's hard to recommend a good way to approach something when what that something is trying to do is masked by poor names that obscure the meaning.

Consider the difference in readability between:

boolean sdsd = asdsda.ahjgsjdgsja();

and

boolean canTeleport = person.isWearingARedHat();

With all do respect, give me a break. For a language that allow statements like - Welcome welcome = new Welcome; or that has to ask programmers to remember to Capitalize the names of Objects so the next guy and tell its an object shouldn't be throwing stones on a variable name of mTrim which is defined to be the trimmed version of "m" via m.trim. As for the setIh, etc., if a person can read Java's gibberish they should be able to figure out that "setIh" is setting the index of the Handicap column.

let's move on.

Do you have any ideas on how to use List / linkedList to figure out is a list of "name", "ghin #", "h.i." has any or all of its elements in a second list, like one build from the input token from a record of the textfile. I'm sure this can be done, but wonder how I'd know the columns the matches came from.

[Jeff Verdegan]

Chuck Beauregard wrote:With all do respect, give me a break. [...] if a person can read Java's gibberish they should be able to figure out that "setIh" is setting the index of the Handicap column.

You have asked people to volunteer their time to help you.

One of those people volunteered his time to try to help you, and gave you a suggestion that would A) in the immediate term make it easier for him and others to provide that help you're seeking, and B) if developed as a habit, make it easier for you and others (such as teammates or those whose help you may seek in the future) to understand the code that you write in general.

You then dismissed that suggestion out of hand, and told your would-be helper what he "should" do, in a rather rude tone.

That approach is not likely to encourage people here to want to help you.

[fred rosenberger]

"Always write code as if the next person reading it were a homicidal maniac who has your home address".

Sure, perhaps today you remember that "setIH" means "index handicap". But in six months, or two years, when you have code that is literally thousands of lines long, will you remember? Will you remember it is "item handicap" or "index hotel" or "internal homework" or something else?

Bear has been programming for a long time. He probably knows a few things about what makes code easy to read and what doesn't.

[Campbell Ritchie]

fred rosenberger wrote:"Always write code as if the next person reading it were a homicidal maniac who has your home address". . .

Any idea where that quote is from. I googled it and didn't find its origin.

[Matthew Brown]

Campbell Ritchie wrote:Any idea where that quote is from. I googled it and didn't find its origin.

There’s a saying we have in the software world, and it will serve you well to remember it while building your Developer project: "Code as if the next guy to maintain it is a homicidal maniac who knows where you live."

SCJP Study Guide (eBook version), Sierra & Bates, p.809. Don't know where they got it from, though.

[Campbell Ritchie]

Amazon have a K&B SCJP study guide going back to 2005, and there are probably older versions around second‑hand. That is older than the oldest quote I found, which was a coding horror dated 2008.
Thank you.

[Matthew Brown]

Well, my edition is 2009, so I can't confirm when it was first used. It's in the additional SCJD chapters available on the eBook but omitted from the main text.

[Chuck Beauregard]

Jeff Verdegan wrote:
You have asked people to volunteer their time to help you.

One of those people volunteered his time to try to help you, and gave you a suggestion that would A) in the immediate term make it easier for him and others to provide that help you're seeking, and B) if developed as a habit, make it easier for you and others (such as teammates or those whose help you may seek in the future) to understand the code that you write in general.

You then dismissed that suggestion out of hand, and told your would-be helper what he "should" do, in a rather rude tone.

That approach is not likely to encourage people here to want to help you.

Thank you for your feedback. Point taken.

Email or BB don't lend themselves well to multiple threads. In a conversation about "classes" and "methods" - nouns and verbs, I shouldn't have put in a piece of code I didn't like, interjecting a second topic. My bad. Causing a negative reaction when the reply was only about the "bad code". Not about classes and method, the topic I was really interest in.

My apologies to "Bear".

[Jeff Verdegan]

Chuck Beauregard wrote:
Thank you for your feedback. Point taken.
[...]
My apologies to "Bear".

While I can't speak for Bear, for my own part, thanks for taking the feedback positively and moving past it.

Now, if we can focus on your problem:

Users will need to enter their current membership roster with handicap data to start the program. This data is available from an independent (very independent) source called GHIN. GHIN has standard reports for each club, which are not very standard at all, and are NOT modifiable. These reports can be exported in a number of ways, Excel, Text, PDF to name a few.

Knowing what data is available, and what it represents in an abstract or logical sense, figure out what OO model you would use for this data, without regard to what form it originates in. Then you can work in from the other direction, figuring out how to parse it into some not-necessarily-OO model that's close to it's native format, then bridge the gap between those two.

For this, I might see 4 different classes--your desired OO model of the data, a model representing the original format, a parser to turn the file or stream or whatever into the original format model, and a transformer to turn that model into your OO model.

This is certainly not the only, possibility, and probably not even the best. But it should give you a starting point for how to look at the process and think about what classes you might want. In particular, you may find that you don't need two models and a translator. Maybe just a parser and a model.

The issue is the report has a header of varying sizes, it could be 1 line with only "Name", "H.I." and "GHIN" or a subset of them, OR the header could be several lines long. I seen examples with 8 lines of header and footer. These headers appear on each page of the report. In addition the space between the informational column is random. To be effective my program must handle all of them seamlessly.

You may want a Parser interface, with several different classes that implement it, one for each format the file can take.

Then you may want a ParserFactory or Configurator or somesuch class that does an initial pass of the file to figure out which Parser implementation to use, and instantiate the appropraite one.
Or that may be overkill. Maybe it's just one Parser class with a "figure out the format" method and a handful of "parse format x, y or z" methods.

The point being, however you realize it in code, "determine which format it's in" is one step you'll have to perform, so that's probably one high-level method, and "parse it according to that format" is another step you'll have to perform, so another high-level method. Those methods may be in the same class, or maybe in different classes. A beginner will be tempted to put them all in one class, thinking it's wasteful or too much trouble to create a bunch of small classes. An experienced OO programmer knows that a bunch of small, simple classes is generally preferred. I personally have a tendency to overengineer things, so I might be tempted to take the breakdown to classes and interfaces too far. In my experience though, it's easier to recover from that by combining classes than it is to split things up once you've mashed too much into one class.

Again, don't take any of this as gospel. There are multiple ways to approach a problem. I'm just throwing out some example of how you might break the responsibilities down, based on my own OO experience and what you've told us about your requirements. These approaches are meant as examples, and to help you see what kind of thinking goes into building an OO solution.

[Chuck Beauregard]

Thanks Jeff for the response.

To restate what I think you are saying, I would in a simple form write it like this.

Input file >>>>( read )>>>>>( Parser )>>>>>[interface route to appropriate - format] >>>>>>>Membership

While my programming days were most likely before you were born and the overhead to call Functions and Subroutine was of concern, I still used them extensively to make the code readable.

I think my current issue is getting a better grasp on the movement of data from the various combinations of classes, methods, packages, objects, instance variables, arrays, lists, parameters, interfaces etc.. The fact that Java passes values in most cases and reference in others is starting to sink in. When I first read about interfaces my reaction was this is for me, now I have to figure out how to actually implement it, in my application.

[Jeff Verdegan]

Chuck Beauregard wrote:The fact that Java passes values in most cases and reference in others is starting to sink in.

Java always passes by value.

Primitives are passed by value.
References are passed by value.
Objects are not passed at all.
Nothing is passed by reference.

And, as you may recall from your "old-timer" days there is an important difference between passing a reference by value and passing an object by reference, although they can look similar in some respects.

And yeah, as you seem to realize, saving every precious byte and CPU cycle is not really on the table with modern OO programming. One of the mantras is write dumb code. Not that we're intentionally wasteful. Just that we recognize the relative cost of hardware vs. programmer-hours.

[Winston Gutkowski]

Chuck Beauregard wrote:While my programming days were most likely before you were born and the overhead to call Functions and Subroutine was of concern, I still used them extensively to make the code readable.

And you shouldn't stop. The only difference is that these days they're called "methods" and they have names. And I doubt very much if you were programming before I was born.

I think my current issue is getting a better grasp on the movement of data from the various combinations of classes, methods, packages, objects, instance variables, arrays, lists, parameters, interfaces etc..

Yup, there's all that; and with OO programming it's even more important, because one of its basic tenets is: Don't re-invent the wheel.

There is one thing about programming though that hasn't changed since I was starting out 40 years ago:
Isolate the problem.

Your first order of business is to translate that report. Where it comes from and how you read it in are incidental to that process. It's a bunch of lines of text that you need to translate so, if it were me, the first thing I'd do is write down a method signature. Perhaps something like:

public {something} translate(String[] lines) {

or (possibly even better):

public {something} translate(List<String> lines) {

Now: What should it return? (ie, what is '{something}')... Do you see where I'm going? Work out your problem incrementally. You may even find you want to put it into a Translator or Extractor class (I would).

All that JFileChooser stuff is just a red herring - and probably a distraction.

Deal with the problem at hand.

HIH

Winston

[Jesper de Jong]

Chuck Beauregard wrote:My apologies to "Bear".

Bear is really his real name.

[Chuck Beauregard]

Winston Gutkowski wrote:[And I doubt very much if you were programming before I was born.

Well at least you sound young! I go back to Control Data 160A/G and 1604 days of the mid-60's. My how time flies.

I often say that any "any asset at some point is a liability and vice versa", Java' reusable code is one of those items to me. I love the idea of reusable code. The issue is finding what you need in the sea of stuff is somewhat overwhelming. I'm currently off to reread about the Collections Framework in general and <List> specifically.

Thanks for the help.

[Winston Gutkowski]

Chuck Beauregard wrote:I love the idea of reusable code. The issue is finding what you need in the sea of stuff is somewhat overwhelming. I'm currently off to reread about the Collections Framework in general and <List> specifically.

A good start.

A few other things come to mind as advice for a procedural bod such as yourself:

1. Remember that the keyword in 'Object-Oriented' is "object". It's the primary unit of construction, and classes are the primary unit of design. You can't do anything substantial in Java without them, and most good OO solutions will have several working together.

2. When you're putting together a solution - especially at the early stages - try NOT to code, or even think in code. Concentrate on what you need to do, not how you're going to do it.
It's probably the single most difficult thing for a programmer brought up in the procedural world to do (BTDTBTTS ) and it's something that we constantly have to get beginners here out of the habit of doing. You may find the StopCoding (←click→) and WhatNotHow pages useful as background reading.

3. Avoid the static keyword. You'll like it, because it allows you to design classes and methods that look and behave very much like the old functional programs that you're probably familiar with but, with a few rare exceptions, it's bad practise. Obviously, you can't eliminate it completely (main() for example, must be static), but try to avoid it as much as possible, especially while you're learning.

HIH, and good luck.

Winston

[Winston Gutkowski]

Winston Gutkowski wrote:A few other things come to mind as advice for a procedural bod such as yourself:...

Oops. One I missed:

4. Don't worry about memory. Specifically:

Don't worry about the amount of memory you're using. It's cheap these days.

Don't worry about what objects look like, or where they're put. That's Java's job.
       Create objects → use 'em → let 'em go.
    Unless you start getting "clever", the garbage-collector will handle the rest. It's a wonderful invention - and very liberating.

HIH

Winston