Skip to content

Added RemoveVowel and tests for issue 20.#23

Merged
ZakBibi merged 3 commits intodevelopfrom
issue_20
Nov 13, 2018
Merged

Added RemoveVowel and tests for issue 20.#23
ZakBibi merged 3 commits intodevelopfrom
issue_20

Conversation

@ZakBibi
Copy link
Copy Markdown
Owner

@ZakBibi ZakBibi commented Nov 12, 2018

No description provided.

@ZakBibi ZakBibi requested a review from BasilBibi November 12, 2018 16:51
def test_01_remove_vowels_from_word_starting_with_vowel(self):
word = 'abacus'
self.assertEqual('abcs', remove_vowels(word))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Any more tests you can think of ?

vowels = ['a', 'e', 'i', 'o', 'u', 'A', 'E', 'I', 'O', 'U']
abbreviation = []
abbreviation.append(word[0])
for w in word[1:]:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there a way to turn this for loop into a list comprehension ?

Copy link
Copy Markdown
Owner Author

@ZakBibi ZakBibi Nov 13, 2018

Choose a reason for hiding this comment

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

I did try this as a list comprehension, and it looks like this:

[abbreviation.append(w)
     for w in word[1:]
     if w not in vowels]

I do not think it makes it any easier to read.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Agreed.

Copy link
Copy Markdown
Collaborator

@BasilBibi BasilBibi left a comment

Choose a reason for hiding this comment

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

Ok this is ready to be merged to develop good work.

@ZakBibi ZakBibi merged commit dba0bba into develop Nov 13, 2018
@ZakBibi ZakBibi deleted the issue_20 branch January 23, 2019 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants