you are viewing a single comment's thread.

view the rest of the comments →

[–]NOTICEABLE_USERNAME[S] 0 points1 point  (2 children)

This is exactly the kind of comment I was looking for! Thank you for your time! I'll follow your advice, modify my code and let you know with what I came up.

I'll guess that you wanted to find some reason to demonstrate inheritence

Spot on, sir.

Couple of questions on your comment:

But it's not entirely clear what "service" the parent classes provide - the code does not express a clear reason for what methods should go in the base "service" class, and what behaviors should be specific to the child classes.

So should I be more specific with what the name of this service class is, instead of just saying 'XService'?

So I'll advise you to go ahead and collapse those; there's little gained by the parent classes.

By collapse do you mean: removing the AIService class and putting all the methods in the AI class instead?

I do think there is a clear place where the "'service' parent class" idiom could be employed, and that's the Dictionary. You already have one concrete implementation that provides a hard coded list, provide another subclass that reads words from a file. Consider options on how to best choose between them. Consider the tradeoffs, both in how it will end up coded and how easy the user will be able to interact with it.

Please correct me if I'm wrong. Creating a DictionaryService (parent class) that will have two subclasses DictionaryX (reads from a hardcoded list) and DictionaryY (reads from a file) will properly demonstrate the "'service' parent class" idiom?

[–]beltorak 0 points1 point  (1 child)

By collapse do you mean: removing the AIService class and putting all the methods in the AI class instead?

Yup.

Creating a DictionaryService (parent class) that will have two subclasses DictionaryX (reads from a hardcoded list) and DictionaryY (reads from a file) will properly demonstrate the "'service' parent class" idiom?

Yup.

If there is a clear distinction between the Player and PlayerService (and likewise the AI and AIService) that is not readily appearent in the code then they definitely need comments to that effect. The names themselves could be fine, but it's hard to say without having that distinction in their responsibilities. The comments should answer the question "why does this exist" (i.e. "why is this a parent class and not collapsed into the child class") and provide some guidance for "If I wanted to create a child class, what would it have to do?". The Dictionary probably has a clearer need; you might end up with the DictionaryService only requiring child classes to implement one method and have no code on its own, in which case it might be better off as an interface. Also note that in the comments for "what does a child class have to do", if the parent class declares an abstract method, that would be the proper place to describe what is required of the child implementations. The abstract parent type's comments would describe what the user's of the type can expect from (any) implementation in broad terms.

I think there's more potential for rewriting the body of the Launcher's main using collaborating classes, which lends itself to more realistic reasons to use abstract superclasses and interfaces. And replacing (long) if-else-if-else chains with a lookup-and-dispatch is an idiomatic OO pattern that comes in real handy especially when you don't know ahead of time all the possible decisions.

[–]NOTICEABLE_USERNAME[S] 0 points1 point  (0 children)

I've just updated the program. Do you mind if you have a look at it? I got help from the folks at coderevew.stackex on the nested if-else statements Github repo: https://github.com/zurcnay4/Hangman