Skip to content
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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Mar 7, 2017
Merged

pangram: add starter implementation and make object-based #344

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
126a6c1
pangram: make object-orientated
FridaTveit Mar 6, 2017
87b340c
pangrams: add starter implementation
FridaTveit Mar 6, 2017
581ae06
Merge branch 'master' into PangramChangeToObject
FridaTveit Mar 6, 2017
442f498
pangrams: rename class to PangramChecker
FridaTveit Mar 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions exercises/pangram/src/example/java/Pangrams.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
public class Pangrams {
private static final int alphaLength = 26;

public static boolean isPangram(String input){
return Arrays.stream(input.replaceAll("[^a-zA-Z]","").toLowerCase().split(""))
public boolean isPangram(String input) {
return Arrays.stream(input.replaceAll("[^a-zA-Z]", "")
.toLowerCase()
.split(""))
.collect(Collectors.toSet())
.size() == alphaLength;
}
Expand Down
Empty file removed exercises/pangram/src/main/java/.keep
View file
Open in desktop
Empty file.
6 changes: 6 additions & 0 deletions exercises/pangram/src/main/java/Pangrams.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
public class Pangrams {
Copy link
Contributor

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?

Copy link
Contributor Author

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?

Copy link
Contributor

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?

Copy link
Contributor Author

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 :)

Copy link
Contributor

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.

Copy link
Contributor

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. 😄


public boolean isPangram(String input) {
throw new UnsupportedOperationException("Method has not been implemented yet.");
}
}
33 changes: 20 additions & 13 deletions exercises/pangram/src/test/java/PangramsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,75 +1,82 @@
import org.junit.Test;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;

import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

public class PangramsTest {
private Pangrams pangrams;

@Before
public void setup() {
pangrams = new Pangrams();
}


@Test
public void emptySentenceIsNotPangram() {
assertFalse(Pangrams.isPangram(""));
assertFalse(pangrams.isPangram(""));
}

@Ignore
@Test
public void pangramWithOnlyLowerCaseLettersIsRecognizedAsPangram() {
assertTrue(Pangrams.isPangram("the quick brown fox jumps over the lazy dog"));
assertTrue(pangrams.isPangram("the quick brown fox jumps over the lazy dog"));
}

@Ignore
@Test
public void phraseMissingCharacterXIsNotPangram() {
assertFalse(Pangrams.isPangram("a quick movement of the enemy will jeopardize five gunboats"));
assertFalse(pangrams.isPangram("a quick movement of the enemy will jeopardize five gunboats"));
}

@Ignore
@Test
public void anotherPhraseMissingCharacterXIsNotPangram() {
assertFalse(Pangrams.isPangram("the quick brown fish jumps over the lazy dog"));
assertFalse(pangrams.isPangram("the quick brown fish jumps over the lazy dog"));
}

@Ignore
@Test
public void pangramWithUnderscoresIsRecognizedAsPangram() {
assertTrue(Pangrams.isPangram("\"the_quick_brown_fox_jumps_over_the_lazy_dog\""));
assertTrue(pangrams.isPangram("\"the_quick_brown_fox_jumps_over_the_lazy_dog\""));
}

@Ignore
@Test
public void pangramWithNumbersIsRecognizedAsPangram() {
assertTrue(Pangrams.isPangram("\"the 1 quick brown fox jumps over the 2 lazy dogs\""));
assertTrue(pangrams.isPangram("\"the 1 quick brown fox jumps over the 2 lazy dogs\""));
}

@Ignore
@Test
public void phraseWithMissingLettersReplacedByNumbersIsNotPangram() {
assertFalse(Pangrams.isPangram("\"7h3 qu1ck brown fox jumps ov3r 7h3 lazy dog\""));
assertFalse(pangrams.isPangram("\"7h3 qu1ck brown fox jumps ov3r 7h3 lazy dog\""));
}

@Ignore
@Test
public void pangramWithMixedCaseAndPunctuationIsRecognizedAsPangram() {
assertTrue(Pangrams.isPangram("\"Five quacking Zephyrs jolt my wax bed.\""));
assertTrue(pangrams.isPangram("\"Five quacking Zephyrs jolt my wax bed.\""));
}

@Ignore
@Test
public void pangramWithNonAsciiCharactersIsRecognizedAsPangram() {
assertTrue(Pangrams.isPangram("Victor jagt zwölf Boxkämpfer quer über den großen Sylter Deich."));
assertTrue(pangrams.isPangram("Victor jagt zwölf Boxkämpfer quer über den großen Sylter Deich."));
}


@Ignore
@Test
public void panagramInAlphabetOtherThanAsciiIsNotRecognizedAsPangram() {
assertFalse(Pangrams.isPangram("Широкая электрификация южных губерний даст мощный толчок подъёму сельского хозяйства."));
assertFalse(pangrams.isPangram("Широкая электрификация южных губерний даст мощный толчок подъёму сельского хозяйства."));
}

@Ignore
@Test
public void upperAndLowerCaseVersionsOfTheSameCharacterShouldNotBeCountedSeparately() {
assertFalse(Pangrams.isPangram("the quick brown fox jumped over the lazy FOX"));
assertFalse(pangrams.isPangram("the quick brown fox jumped over the lazy FOX"));
}
}