• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Basic accountancy project

 
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I'm a little lost for what to do in this exercise. Basically, I have three classes: Accountant, Client, Contract and AccountingDriver. It is supposed to be able to do the following:

1. Create a new accountant and add it to the list of accountants.

2. Create a new client and add it to the list of clients

3. Process a contract request. When a client requests an accountant in a specific area of expertise, the system should first check that there is an accountant with the required expertise that is not on another contract at the moment. If there is an available and suitable accountant then a contract is created for a particular duration (number of days), at a daily rate that depends on the particular accountant being contracted.

4. Process a contract completion. When a contract is complete, the amount owed by the client should be computed and the accountant should again be indicated as available for further contracts.

5. The system should be capable of generating a report of all currently active contracts. For each contract, the report should list the details of the accountant, client, duration and the daily rate.


I've printed the code for the driver class below.





So far all I could think of doing for the requestContract method was something like


I'm not at all sure where to go from there though. I don't even know how I should mark an accountant as engaged. My first instinct is to create an extra attribute for the class of type boolean isEngaged, but I don't think that's what's expected of me since it isn't specified in the accountant class.

Just to be clear, I'm not asking for any actual solutions to my problem. All I want is a nudge in the right direction.
 
Bartender
Posts: 3648
16
Android Mac OS X Firefox Browser Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Anna Greene wrote:I'm not at all sure where to go from there though. I don't even know how I should mark an accountant as engaged. My first instinct is to create an extra attribute for the class of type boolean isEngaged, but I don't think that's what's expected of me since it isn't specified in the accountant class.

Just to be clear, I'm not asking for any actual solutions to my problem. All I want is a nudge in the right direction.



Welcome to the Ranch.

Adding a new variable "engaged" is what I would do. Once you checked the accountant's expertise matches, set this engage boolean flag on and exit the loop. Also remember to use the equals() method when comparing strings rather than the == operator.

 
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Anna Greene wrote:I'm not at all sure where to go from there though. I don't even know how I should mark an accountant as engaged. My first instinct is to create an extra attribute for the class of type boolean isEngaged, but I don't think that's what's expected of me since it isn't specified in the accountant class.


Well, I think there are two things here. What K.Tsang has showed you is how to find an "available" Accountant; whereas I think what you want is how to mark an Accountant as "engaged" and have them stay engaged until they've completed their contract.

There are several possibilities:
1. (which is I think what you were thinking) Add an 'engaged' attribute to the Accountant class. Personally, I think you're misgivings are right: it isn't really an attribute of an Accountant; it's something we (the company) want to use to flag him/her as "busy".
2. Set up a parallel ArrayList of Booleans that indicates whether the Accountant with the same index is "engaged".
3. Exactly as (2), but use a BitSet (a VERY useful class to know about).
4. (The most OO in my view) Create a new class CompanyAccountant that wraps or extends an Accountant, and adds an 'engaged' attribute.

Unfortunately, I don't know if the AccountingDriver class has been supplied to you and, if so, how much you're allowed to change it; but (4) would definitely be my first choice, followed by 3 then 2 then 1.

HIH

Winston
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:4. (The most OO in my view) Create a new class CompanyAccountant that wraps or extends an Accountant, and adds an 'engaged' attribute.


Actually, looking at your requirements, you might think about creating a Contract class instead, and use that to do your "engaged" check.

Let's suppose that an Accountant can only work on one Contract at a time. You could then hold all your Contracts in a structure that keys them by Accountant, and allows you to quickly check their latest Contract; eg, something like a:
LinkedHashMap< Accountant, LinkedList<Contract<Accountant, Client>> >

Now that probably looks like a huge mouthful, but all it is is a glorified HashMap, where each "value" is a list of contracts for an Accountant. The reason the list is a LinkedList is that you can easily check their latest Contract with its getLast() method.

Now, how you determine whether the Accountant is "engaged" on the Contract would be up to you. You could use start/end dates or some sort of status (pending/started/completed) - I leave it to your inventiveness.

It may seem more complicated than the previous suggestion, but I think you may find it fits better with your requirements.

HIH

Winston
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Anna Greene wrote:3. Process a contract request....


Just thought of one other thing. There's a subtle problem lurking in this requirement - although it's probably beyond the scope of your class right now.

If you simply go through your list looking for a suitable - and available - accountant, doesn't that mean that those at the beginning of the list will tend to get more work than the ones at the end? Seems a bit unfair.

Once you've got the rest of it working - and only then - have a think how you might give every accountant a fair crack at getting a contract.

Winston
 
Anna Greene
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:

Winston Gutkowski wrote:4. (The most OO in my view) Create a new class CompanyAccountant that wraps or extends an Accountant, and adds an 'engaged' attribute.


Actually, looking at your requirements, you might think about creating a Contract class instead, and use that to do your "engaged" check.



Contract is one of the classes. Here's what it looks like as it was handed out.



While I said above that there are three classes, there are actually four: Accountant, Client, Contract and AccountingDriver. Psychologically, I tend to overlook driver classes quite a lot when talking about things at a class level. If I use Contract for the engaged "attribute", how would I link it to accountant though? From my understanding of OO philosophy having either one of those classes "extend" the other would make no sense, so I gather that's not what you mean.
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Anna Greene wrote:Contract is one of the classes. Here's what it looks like as it was handed out.


Ah. It would have been good to know this at the start.

However, just looking at it, I can see a glaring omission: unless this class is a template for a Contract, where is the Client? Surely someone has to sign that Contract?

If I use Contract for the engaged "attribute", how would I link it to accountant though? From my understanding of OO philosophy having either one of those classes "extend" the other would make no sense, so I gather that's not what you mean.


You're right. Let's assume that you can't change those entity classes - so, what about an Assignment class that ties a Contract, Client and Accountant together? In English terms: an Assignment is the fulfillment of a Contract for a Client by an Accountant.

So it might look something like this:I leave the rest up to you.

Now you could create a Map of Assignments for each Accountant as I suggested above:and your "engaged" check in your loop might look something like:Do you follow how that works?

If an Accountant doesn't have a "current" assignment, or they've finished it, they're "free"; otherwise they're "engaged" - which is probably close to how it actually works in real life. The Map is just to give us quick access to their Assignments.

To be honest, the LinkedHashMap may be overkill - a regular HashMap would do just as well for your "engaged" check - but they can be very useful.

Also: This is only ONE possibility. There are many others.
The thing I like about it is that it uses the classes you've been given in a way that models the way an accounting firm might actually work; but it's certainly more involved than simply adding an 'engaged' attribute to your Accountant class.

But you're the one writing this, so you have to decide what's best.

HIH

Winston
 
Anna Greene
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I'm really starting to think this project is above me. We haven't even covered LinkedLists or HashMaps yet. I'm just not making progress here at all. Every time I think I have a great idea, I realise it won't work within a short period of time. I'm not at a level where I can really implement most of the suggestions already given. I thought this was more basic than it actually is. It's actually really far above everything else we've been doing so far.
 
Anna Greene
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Okay, this assignment is long over, but I never quite managed to get it working properly. I used a hash map which allowed it to work for the very first contract, but wouldn't work for anything after that. I simply opted for not looping it instead, which still met the project criteria, but it felt like a cop out. Now that I have some free time, I'll show you the project I sent in in its entirety. Here are all four classes below:









If you spot any area where I have used some feature improperly, please mention it. Any suggestions on how it could be improves would also be welcome.
 
Marshal
Posts: 79177
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Please don't use such long lines; they make it difficult to read your code. And use Tabs not spaces for indenting. That is why my corrections to your long lines didn't make the code look any better. More explanation here. If I had been marking, those two style errors would have lost you 1 mark each. You would have lost another mark for having {} on the same line.
You obviously know how to write classes.
Why have you used Strings for sphere of expertise, rather than an enum?
Why have you used a double for money? (Though many programming teachers do not know this is an error.)
Why have you given set methods for the ID? Surely the ID won't change and it should be final?
And why did you not initialise the engaged field (better than isEngaged) in the constructor?
Only use \n or \r in String literals if somebody has told you they want an LF or CR character. Otherwise, use println or printf…%n instead.
Never never use == true, == false, etc. It is if (b)… or if (!b)… Using == is poor style and error‑prone; we see lots of people who write = by mistake and then you have all sorts of potential errors.
Declare your Lists as List<…> and instantiate them as ArrayList<>, similarly for Maps.

The reason you can only get one contract into a Map is that a Map represents a function. You can read about it in the API documentation and in the Java™ Tutorials. That means one man, one vote one key, one value. If you “put” Jack Smith as accountant twice, things may start to go wrong. If you have the same client, you get no change, otherwise you provide him with a new client and lose the old one. I think what you have is a class which matches an accountant to a client, so you can have several instances of that class, maybe putting them in a List and maybe taking them out when the job is completed.
There is something peculiar about those methods which create a new Object and immediately add it to a List; their intent is not obvious when you read their invocations, maybe this would be better:-I do not like the multiple if…elses. You can use a switch…case with enum elements (or in Java7, Strings) as the keys. Even better, if you use an enum for contract type, you can have all the requisite information in the enum elements, and use that in the contract object.
 
Anna Greene
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
That is an absolutely fantastic reply Campbell. I appreciate that it's straight and to the point. You mentioned pretty much everything that had me confused. I still need to go through the documentation you linked, but I'm pretty sure I'll be able to fix the mess I made now. However, the people teaching me actually recommended calling booleans isX or isY, so I'm surprised to hear that isn't standard practice.
 
Campbell Ritchie
Marshal
Posts: 79177
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Anna Greene wrote:That is an absolutely fantastic reply Campbell. . . .

You're welcome . I had thought you were going to tell me off for being hard on you.

However, the people teaching me actually recommended calling booleans isX or isY, so I'm surprised to hear that isn't standard practice.

Anybody else like to reply about this, in case I am mistaken?

As far as I can remember, the standard practice is to call booleans active, finished, engaged, enabled, etc. The standard practice is to call methods with a boolean return type isActive, isEnabled, hasFinished (or isFinished), isEngaged, etc. You will find most such methods start with is…, but you will often see has… or can…
These conventions are old, but as far as I know, still valid. But they don't say anything about is. This Wikipedia page might be more helpful.
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Anna Greene wrote:However, the people teaching me actually recommended calling booleans isX or isY, so I'm surprised to hear that isn't standard practice.


Are you sure? Or did they say that you should start the name of a getter method for a boolean with "is"? That makes a lot more sense (and it's actually the standard in Eclipse).

The only things I can add to Campbell's comprehensive dissection are:
1. Always, always, always, make instance fields private. Your accountantContracts field isn't. If other classes in your app need access to it, then provide a getter with the relevant visibility, but DON'T expose fields.

2. Your classes seem overly reliant on code to do their job. I can't remember the last time I wrote a method with more than about 30 lines in it, but you have one with 80. It's probably my database/modelling background, but I always try to think about what db tables I'd need to store the information in an app when I'm thinking about what Java classes I'm going to need.

3. Your Contract class doesn't contain an Accountant object. Surely the work has to be done by somebody? And isn't who does it likely to be the determining factor (or one of them) in the dailyRate?

4. I agree completely with Campbell about big if...else lists, but I'd go even further: they are dispatch code (look it up), and dispatch code is often symptomatic of something that could be solved with polymorphism (ie, with subtypes of Contract), rather than if...else (or switch). Even as it stands, your if blocks contain lots of redundancy.

HIH

Winston
 
Sheriff
Posts: 5555
326
IntelliJ IDE Python Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Campbell Ritchie wrote:

However, the people teaching me actually recommended calling booleans isX or isY, so I'm surprised to hear that isn't standard practice.

Anybody else like to reply about this, in case I am mistaken?


I would not prefix my boolean names with is. But as a matter of extreme coincidence I have just read the following paragraph in Steve McConnell's Code Complete:

Steve McConnell wrote:Some programmers like to put Is in front of their boolean names. Then the variable name becomes a question: isDone?, isError?, isFound?, isProcessingComplete? Answering the question with true or false provides the value of the variable. A benefit of this approach is that it won't work with vague names: isStatus? makes no sense at all. A drawback is that it makes simple logical expressions less readable: if ( isFound ) is slightly less readable than if ( found ).


In my experience the convention for Java code is not to use the is prefix on your boolean names. But as with all conventions consistency is key so if you're working on a codebase that extensively uses the is prefix then I would use it too.
 
Campbell Ritchie
Marshal
Posts: 79177
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote: . . . and dispatch code is often symptomatic of something that could be solved with polymorphism (ie, with subtypes of Contract), rather than if...else (or switch). . . .

I was thinking the same sort of thing when I suggested enums. If you have an enum element which has embedded data. then subtyping Contract may become redundant.
 
With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime.
reply
    Bookmark Topic Watch Topic
  • New Topic