Campbell Ritchie wrote:Why have you given the Lake a static field and static methods?
Ignore it for the time being.Bren Reggy wrote: . . . My problem is now the threads each instantiate a seperate FishingRod and Lake object (as if they each are fishing in their own lakes!). It is obvious that this is cause the Lake and Rod constructors are in the run() method. I have tried to move this to the main() but the problem is that the run() doesn't recognise "thisLake" object when it's been created in the main(). Probably a silly overlook but how would I resolve this error?
Campbell Ritchie wrote:
You should remove all mention of threads from the Lake class. That is a shared resource, so you shouldn’t care at this stage which thread catches a fish. Think in real life: you have several people catching fish in the same lake. The change is that the lake loses fish (until you put them back), but it cannot tell who has caught what.
Campbell Ritchie wrote:
Get that Lake class working in a one-threaded environment where you have one Fisher and one Rod.
Campbell Ritchie wrote:
Some other changes I would suggest:Your isEmpty method should return a boolean which tells you whether you have 0 fish left. Nothing else.
Campbell Ritchie wrote:
You can simplify the method which removes fish to this: fish--; You don't need the assignment.
Campbell Ritchie wrote:
You can probably lose the newFishAmount field altogether. You are not using it at all.
Campbell Ritchie wrote:
Your getXXX methods should return whatever they get. The getLakeName method should return the name of the lake, and if you want to display it you should write a displayLakeName method.
Campbell Ritchie wrote:
I recommend you write the standard methods for all objects, at least toString.
Campbell Ritchie wrote:
I think the field would be better called name than lakeName, you don't need a reminder that you are in the Lake class.
Campbell Ritchie wrote:
I cannot make similar comments about the Rod and Fisher classes, mainly because I haven't bothered to read them! I presume the many print statements are there for testing and debugging use (good idea ) and you won't use them in the final product.
Campbell Ritchie wrote:
Now, this is the hard bit. Consider what changes you would have to make to the Lake class to make it safe for threaded use. Consider which if its fields will change and which not. If any fields don't change, can you make them final and avoid any threading problems for those fields. If any fields are going to change, how can you make access to them safe for threaded use.
Campbell Ritchie wrote:
Bren Reggy wrote:
. . . My problem is now the threads each instantiate a seperate FishingRod and Lake object (as if they each are fishing in their own lakes!). It is obvious that this is cause the Lake and Rod constructors are in the run() method. I have tried to move this to the main() but the problem is that the run() doesn't recognise "thisLake" object when it's been created in the main(). Probably a silly overlook but how would I resolve this error?
Ignore it for the time being.
Campbell Ritchie wrote:
I presume you have some method of working out which Fisher has a Rod, when he picks up and replaces the Rod, and when it is or is not available for somebody else to take a turn.
Don't get me started about those stupid light bulbs. |