• Post Reply Bookmark Topic Watch Topic
  • New Topic

Make program more object oriented?  RSS feed

 
Jeremy Pierson
Greenhorn
Posts: 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello everyone,

I just finished my first semester in Java and for my final project I decided to create a program that would read an excel sheet, sort the data, and output the new data to a new excel sheet. I got the program to work and my professor loved the idea. He said I could make the project easier by making it more object oriented. I wasn't able to get him to elaborate on specifics before the semester ended, so I figured I would ask here. I decided to re-do a good portion of the program. Some of the comments are irrelevant, I had to teach myself to use Java with excel, so a few comments are just instructions/reminders.

Here is what I have so far.

Here is the excel sheet I am reading from (minus the names): http://imgur.com/a/dLZbu ; (image tags didn't work)
Here is an example of the output: http://imgur.com/a/2AgT4

Code:





One thing I wanted the program to do, but was having a hard time implementing was a rotation for the employees (rotate the employees through the sections so they wouldn't work the same section multiple times a week). I was thinking I could serialize an array for each employee, for example {0,0,0,0, etc...} with the available sections as the index. The program would deserialize the array, compare the values (lowest value assigns employee to that section, if values are equal (multiple zeros), randomly picks an employee for that section), +1 to the element of that index if employee is assigned to the section, and serialize the array again. This would occur for some period of time (2 weeks?) or deserialization amount and then reset the values back to zero.

I have never worked with serializing objects, so I'm not sure if it's possible. Would this method work?

Thankyou for any feedback!

 
Carey Brown
Saloon Keeper
Posts: 3328
46
Eclipse IDE Firefox Browser Java MySQL Database VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Drop the parens, they are unnecessary. you should become very familiar with operator precedence in order to avoid clutter. On the flip side, parens are encouraged when doing bit manipulation because most programmers are only somewhat familiar with bit operator precedence.

should be
This is really a constant and as such the name should be in all CAPS. You have a number of variables that should be implemented as constants.

The "else" is missing braces and the indentation implies that the 5 statements below are part of the "else" block, which they are not.

As far as OOP goes, look for nouns, e.g. "Employee". That will suggest classes that you will need to make. Look for "is-a" relationship between classes, that will suggest using inheritance (may not apply to your specific application. Look for "has-a" relationships, that will suggest what classes should contain other classes as fields, especially fields that need to be implemented as a collection, e.g. list-of or array-of.

Working with POI is a bit tricky. My inclination is to read the spreadsheet file into classes, do all my work with the classes, then write all the classes back out to a spreadsheet file. Where this gets weird is that the spreadsheet has knowledge of things like column headers. POI will read them into the POI structure but doesn't really give you a way to recreate some of that structure starting from data stored in class instances. The POI structure is also implemented as a sparse array which is not always easily replicated when going from classes back to spreadsheet.

When you break this down into OOP you should end up with only one read() and one write() (or load() and save()) method.



 
Carey Brown
Saloon Keeper
Posts: 3328
46
Eclipse IDE Firefox Browser Java MySQL Database VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I don't know if this site will let you go back and edit your original post but your long lines (anything longer than 80-120 characters) are making your post extremely difficult to read. Please edit and fix if you can.
 
Carey Brown
Saloon Keeper
Posts: 3328
46
Eclipse IDE Firefox Browser Java MySQL Database VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jeremy Pierson wrote:Hello everyone,

I just finished my first semester in Java and for my final project I decided to create a program that would read an excel sheet, sort the data, and output the new data to a new excel sheet. I got the program to work and my professor loved the idea. He said I could make the project easier by making it more object oriented. I wasn't able to get him to elaborate on specifics before the semester ended, so I figured I would ask here. I decided to re-do a good portion of the program. Some of the comments are irrelevant, I had to teach myself to use Java with excel, so a few comments are just instructions/reminders.

Here is what I have so far.

Here is the excel sheet I am reading from (minus the names): http://imgur.com/a/dLZbu ; (image tags didn't work)

Your image link above didn't work either.
Here is an example of the output: http://imgur.com/a/2AgT4

One thing I wanted the program to do, but was having a hard time implementing was a rotation for the employees (rotate the employees through the sections so they wouldn't work the same section multiple times a week). I was thinking I could serialize an array for each employee, for example {0,0,0,0, etc...} with the available sections as the index. The program would deserialize the array, compare the values (lowest value assigns employee to that section, if values are equal (multiple zeros), randomly picks an employee for that section), +1 to the element of that index if employee is assigned to the section, and serialize the array again. This would occur for some period of time (2 weeks?) or deserialization amount and then reset the values back to zero.

I have never worked with serializing objects, so I'm not sure if it's possible. Would this method work?

It's hard to guide you through OO without having more information about the problem domain.

Does a Schedule have Employees? Or does a Schedule have Shifts that have Sections that have Employees? You see where I'm going? You need to break the problem down into relationships between entities.

Code like above may indicate what you want to see in the output spreadsheet but it doesn't give you a logical representation that you can process easily. These strings shouldn't be necessary until you go to write the output. For a logical representation you might consider making an enum of all of the individual cases, e.g. "Breaker Back" or "3", and have a list of those for each section.

Regarding serialization/deserialization (assuming you mean this in the Java vernacular), forget about it. If you have a good logical layout to your classes you should easily be able to copy lists or arrays of data or you can create copy constructors for your own classes to copy them.
 
Jeremy Pierson
Greenhorn
Posts: 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Your image link above didn't work either.


Sorry, try this:  https://postimg.org/gallery/2uip5qyn4/

It's hard to guide you through OO without having more information about the problem domain.

Does a Schedule have Employees? Or does a Schedule have Shifts that have Sections that have Employees? You see where I'm going? You need to break the problem down into relationships between entities. 


That makes sense. I'll give it some thought. Thankyou

.....
Code like above may indicate what you want to see in the output spreadsheet but it doesn't give you a logical representation that you can process easily. These strings shouldn't be necessary until you go to write the output. For a logical representation you might consider making an enum of all of the individual cases, e.g. "Breaker Back" or "3", and have a list of those for each section.

Regarding serialization/deserialization (assuming you mean this in the Java vernacular), forget about it. If you have a good logical layout to your classes you should easily be able to copy lists or arrays of data or you can create copy constructors for your own classes to copy them


I use those arrays more as constants. Depending on the number of employees, those arrays will remain the same, the other arrays are changed depending on other circumstances and are then aligned with the "constant" arrays. I had to look up enum data types, I don't remember using those much. "An enum type is a special data type that enables for a variable to be a set of predefined constants." You're correct, it would make more sense if they were enum lists rather than arrays. I'll work on that.

As for the serialization issue. I'm not really following how the copy lists or copy constructors would work for that. Would the data be saved when the program is closed and re-opened the following day? i.e. Employee1 worked "3" yesterday, therefore they aren't assigned "3" today?

Drop the parens, they are unnecessary. You should become very familiar with operator precedence in order to avoid clutter. On the flip side, parens are encouraged when doing bit manipulation because most programmers are only somewhat familiar with bit operator precedence.


Fixed, not sure what I was doing there.

"OFF" ... This is really a constant and as such the name should be in all CAPS. You have a number of variables that should be implemented as constants.


The "else" is missing braces and the indentation implies that the 5 statements below are part of the "else" block, which they are not.


Fixed, thankyou

As far as OOP goes, look for nouns, e.g. "Employee". That will suggest classes that you will need to make. Look for "is-a" relationship between classes, that will suggest using inheritance (may not apply to your specific application. Look for "has-a" relationships, that will suggest what classes should contain other classes as fields, especially fields that need to be implemented as a collection, e.g. list-of or array-of.
...
When you break this down into OOP you should end up with only one read() and one write() (or load() and save()) method.


I had a hard time thinking of a way to implement inheritance (is-a relationship). I actually had an "Employee" class in the previous version I made, but was planning on re-doing it.



I don't know if this site will let you go back and edit your original post but your long lines (anything longer than 80-120 characters) are making your post extremely difficult to read. Please edit and fix if you can.


It doesn't appear I am able to edit the original post, but here is the cleaned up code:

 
Carey Brown
Saloon Keeper
Posts: 3328
46
Eclipse IDE Firefox Browser Java MySQL Database VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Carey Brown wrote:Regarding serialization/deserialization (assuming you mean this in the Java vernacular), forget about it. If you have a good logical layout to your classes you should easily be able to copy lists or arrays of data or you can create copy constructors for your own classes to copy them...

Jeeremy Pierson wrote:As for the serialization issue. I'm not really following how the copy lists or copy constructors would work for that. Would the data be saved when the program is closed and re-opened the following day? i.e. Employee1 worked "3" yesterday, therefore they aren't assigned "3" today?

Sorry, I may have misinterpreted your intentions for serialization. There's a trick you can do with serialization to copy a complex structure of classes and that is what I thought you were talking about. Now I see you mean serialization for the persistence layer.
 
Carey Brown
Saloon Keeper
Posts: 3328
46
Eclipse IDE Firefox Browser Java MySQL Database VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jeremy Pierson wrote:I had a hard time thinking of a way to implement inheritance (is-a relationship).

You don't necessarily have an "is-a" relationship, but just keep your eye out in case one becomes apparent. As an example, you might have an Employee and a subclass Contractor, in which case you would have Contractor is-an Employee.

Some comments on your Employee class, mostly based on Java programming conventions.

Often when you have a boolean field the name will be prefixed with "is", as in "isPregnant" and "isFullTime". Then the setters would be setPregnant() and setFullTime() and the getters would be isPregnant() and isFullTime().

You have a method getContract() but it returns fullTime. Again, better expressed as isFullTime().

You use the magic number '7', you might want to define a constant "DAYS_IN_WEEK", if that's what you are meaning. Then use the constant.

You have "starts", "finishes", "firstBreaks", and "secondBreaks", all with a size of '7'. A possiblility would be to combine these into a class like "Period"(?, or something more appropriately named). Then have a Period array of length '7'. Perhaps you "week" should be a part of this as well. It's a little difficult here for me to infer your intended usage.



 
Jeremy Pierson
Greenhorn
Posts: 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Carey Brown wrote:
Jeremy Pierson wrote:I had a hard time thinking of a way to implement inheritance (is-a relationship).

You don't necessarily have an "is-a" relationship, but just keep your eye out in case one becomes apparent. As an example, you might have an Employee and a subclass Contractor, in which case you would have Contractor is-an Employee.

Often when you have a boolean field the name will be prefixed with "is", as in "isPregnant" and "isFullTime". Then the setters would be setPregnant() and setFullTime() and the getters would be isPregnant() and isFullTime().

You have a method getContract() but it returns fullTime. Again, better expressed as isFullTime().


I see. To be honest. That class was thrown together pretty quick when the project's due date was coming up. I was not familiar with the boolean convention. That helps. I will keep an eye out for any possible inheritence. Thank you for your feedback. Gives me a bit to work on. Ill check back when I make some progress to see if there are any other areas I need to work on.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!