Skip to content

Extend pangram test suite #294

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

Closed
victorbrender wants to merge 1 commit into from
Closed

Extend pangram test suite #294

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions exercises/pangram/pangram_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ def test_pangram_with_only_lower_case
assert Pangram.is_pangram?(str)
end

def test_pangram_with_underscore_character
str = 'the quick brown_fox jumps over the lazy dog'
assert Pangram.is_pangram?(str)
end

def test_pangram_with_sentence_containing_numbers
str = 'the qu1ck br0wn fox jumps over the lazy dog'
Copy link
Copy Markdown
Member

@kotp kotp Apr 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid 0 and 1 as they are too close in some fonts to l and O. When possible, use distinctively shaped numbers, rather than the possibly visually ambiguous ones.

This becomes aggravated especially when used in places where those ambiguities are replacements of letters that are so similar.

Copy link
Copy Markdown
Member

@Cohen-Carlisle Cohen-Carlisle Apr 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of this test is to replace the letters with a corresponding number and make sure the solution does not think the string is a pangram. With this in mind, I think its ok. Especially if the method name were updated to something like test_letters_replaced_with_numbers.

At worst its an exercise in paying attention to detail, which is a good skill to have as a programmer.

Copy link
Copy Markdown
Member

@kotp kotp Apr 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that the purpose of the test?

I don't think it is, and if so, it is not expressed clearly.

It appears that the purpose is that if there are 26 unique characters and yet it is missing the only letter this is missing, which happens to be an 'i', it should not be classified as a pangram.

For intent, though, we would definitely need to ask @victorbrender since he is the only one that can confirm intent, as the author.

The visual distraction of similarly shaped numbers in the place of letters is simply a distraction. Noting that the 'i' is the only thing missing is the exercise of detail attention that you speak of. It happens here regardless of the visual distraction of the numbers similarity to the letters they are replacing.

refute Pangram.is_pangram?(str)
end

def test_missing_character_x
skip
str = 'a quick movement of the enemy will jeopardize five gunboats'
Expand Down