search

mharrys / othello

0 stars 0 forks source link

Access the score of the game #34

Closed hygerth closed 9 years ago

hygerth commented 9 years ago

A way to do this is making the Nodes observable and letting the Score observe changes, i.e. use inversion of control.

mharrys commented 9 years ago

BoardFactory creates new instances of Nodes, thus you will lose the observers for that node.

mharrys commented 9 years ago

Expanding on what you suggested. We could "wire up" the observers in the OthelloFactory between the Score and all existing Nodes in the game. We could get rid of the BoardFactory, only send in the Board and NodeSwapper to Othello. Instead we could create a NodeSwapper interface, extend to a ClassicNodeSwapper which knows about ClassicNode, it could call upon "mark" which would notify Score about changes.

Still not entierly happy with that solutions though.

hygerth commented 9 years ago

Sounds good, but I agree that it is not perfect. I have searched around on the internet for an solution to this, but everything that I have found are similar solution to what we have with the BoardFactory and worse.

mharrys commented 9 years ago

For instance, do NodeSwapper need to rely on the class Board? I would say no, what if your ClassicNodeSwapper has a constructor, ...(List<ClassicNode> nodes) and a interface swap(List<Node> nodesToSwap...). You would not need to know about Board because board can only give you List<Node> and we do not want to cast it. But this way, we dont even know that there exist a board, we only are interested in pairing up nodesToSwap with the list of nodes we have. We then can invoke "mark".

mharrys commented 9 years ago

Updated comment again, markdown was a bit troubeling.

hygerth commented 9 years ago

That was what I desired when I wrote NodeSwapper, but since we used BoardFactory and I had not figured out an alternative I had to send a board into it.

mharrys commented 9 years ago

Should I give it a try and push it up for evaluation? Or do you have something else in mind?

hygerth commented 9 years ago

Give it a try, since it is a better solution (at least in theory) to only send list of nodes into NodeSwapper.

mharrys commented 9 years ago

You can have a look in the feature-score branch. I have not run the test code but ComputerGame/OnePlayerGame seems to be working. I also left out some documentation.

mharrys commented 9 years ago

Score now is wired up to the nodes with observers.

hygerth commented 9 years ago

Seems to be providing correct results. I have rewritten the test for the NodeSwapper, is it ok to push to this branch?

mharrys commented 9 years ago

Sure, I was wondering how to do them test because ClassicNodeSwapper uses "mark".

mharrys commented 9 years ago

I will see if I can write a test for OthelloScore also.

hygerth commented 9 years ago

Okay. It was smart of you to let the NodeCapture class to include the placed disk, in order to minimise the dependency of NodeFinder.

mharrys commented 9 years ago

I don't think we actually need the NodeFinder? Those methods could be private in NodeCapturer. I think it still would conform to the SRP, or?

mharrys commented 9 years ago

Did a pull request, the score feature is there at least.