-
-
Notifications
You must be signed in to change notification settings - Fork 682
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pangram: add starter implementation and make object-based #344
Conversation
Pangrams is currently the third exercise in the new ordering therefore it should have a starter implementation.
@@ -0,0 +1,6 @@ | |||
public class Pangrams { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It now appears that the object we're dealing with is no longer a "pangram"... it's a ... "sentence"? What would the API look like if we instantiated a sentence and interrogated it as to whether it is a pangram?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean, but I think it would be a bit strange to have a "sentence" object and all we can do with it is ask if it is a pangram. I find it confusing when the class names don't reflect what the exercise is about, which in this case would be pangrams not sentences. I do agree with you that it's not really a "pangram" object though. Maybe PangramChecker? So we instantiate a pangram checker and interrogate it as to whether the provided sentence is a pangram?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... I think it would be a bit strange to have a "sentence" object and all we can do with it is ask if it is a pangram ...
I think totally get that point. We're invoking two concepts instead of just one. And I think you're saying that it feels rather surprising to spend time talking about "pangrams" and then suddenly turn around and start with "okay, so you've got a sentence..." Is this part of where you're coming from?
A switch flipped in my head when I read your commit comment, "make object-oriented." It reminded me that the roots of Java are in the OO model of thinking.
I'm a rather concrete learner, so I have the sketch it for myself...
public class Sentence {
public Sentence(String input) { ... }
public boolean isPangram() { ... }
}
and
public class PangramChecker {
public boolean isPangram(String input) { ... }
}
We can take (at least) two views with the API design: it's a Function Object or it's a Value Object that happens to have one function.
If I see a Function Object, it feels like a ... function: stateless and composed. This style has become much more idiomatic these days with the literal @FunctionalInterface
annotation as well as this kind of structure being a natural outcome of SRP. Also, such shapes fit easily into DI/IoC frameworks like Spring.
When I see an Object, it feels like a wrapper that adapts the contents to something else. A wrapper says, "from these ingredients we have a new thing which has these new property/ies." This style hit the "Object-Oriented" button pretty squarely: it's intended to convey a model in the programmers' minds of "things that have state+behavior".
I suspect there's not enough design pressure from the exercise itself to tip us one way or another. I might have inadvertently drifted into the "stylistic" realm...
What would be most useful to someone at this point on the track?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time to explain that, that's very useful! :)
I think I would prefer it to be a Function Object. That's mostly because if we have a "sentence" object I wouldn't expect it's only function to be "is it a pangram", that's not the first thing I associate with a sentence. So it doesn't really feel like it encapsulates what a sentence is, any more than a string does. We only really want that one function so I think it makes more sense to make it a Function Object.
I also think that it makes it more clear to the person doing the exercise what the exercise is about. I know the README explains that but it's nice if everything is as clear as possible :)
But I'm open to being convinced otherwise if you disagree :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for talking that through with me, Frida. That's solid thinking; I'm totally on the same page with you.
I also like the idea of renaming the class as you mentioned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to chuckle at this. Somewhere in the way back I think there is a discussion between me and @sit with a different, but somehow similar flavor. 😄
See issue #178 and #177.