Skip to content

phoenix class - Anees Quateja#39

Open
aneesquateja wants to merge 4 commits into
Ada-C22:mainfrom
aneesquateja:main
Open

phoenix class - Anees Quateja#39
aneesquateja wants to merge 4 commits into
Ada-C22:mainfrom
aneesquateja:main

Conversation

@aneesquateja

Copy link
Copy Markdown

No description provided.

@apradoada apradoada left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well done, Anees! This project scores a GREEN!

Overall your code looks great and it passes all the tests! I just left a few comments here and there for you to think about, but really nothing that you need to correct unless you would like to! Well done and congratulations on finishing your first project!

Comment thread adagrams/game.py
def draw_letters():
pass
# array_of_strings = [ ]
LETTER_POOL = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is a great dictionary to hold the letters and the count of each letter! When creating it, you've followed the naming convention of a global variable which I agree would be the best way to hold this information just in case we need to use this dictionary elsewhere! The one change I would make here would be to move it outside this function which is what we would do to make sure the dictionary is available to the other functions that might need it. If we have several pieces of this kind of information, it is also possible to put your data in a separate file and import it in as well!

Comment thread adagrams/game.py
}

letter_pool = []
for letter, count in LETTER_POOL.items():

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I love this way of creating a list to hold every letter while still taking into account the distribution! It's short and concise and works well! Great use of the extend methond!

Comment thread adagrams/game.py
for _ in range(10):
random_index = random.randint(0, len(letter_pool) - 1)
hand.append(letter_pool[random_index])
letter_pool.pop(random_index)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This for loop is once again concise and well constructed! Great use of the pop method as well. The only consideration I would add is that the pop method does run in O(n) time. If you'd like an extra challenge, see if you can write this in a way that doesn't use the pop method and runs in O(1) time! Note: That code will be a little more involved but it will decrease the complexity.

Comment thread adagrams/game.py
letter_bank_remaining.remove(letter)
else:
return False
return True

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Love this function! It's easy to read, well written and concise! No notes!

Comment thread adagrams/game.py
pass

# Define the letter scores
score_chart = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If we were going to expand on this project, this dictionary would be a great candidate for global variable status!

Comment thread adagrams/game.py

# Sum up the score for each letter
for letter in word:
total_score += score_chart.get(letter, 0) # Add 0 if letter is not found (for safety)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I like the safety check here!

Comment thread adagrams/game.py
total_score += score_chart.get(letter, 0) # Add 0 if letter is not found (for safety)

# Add bonus points if the word is 7-10 letters long
if 7 <= len(word) <= 10:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great use of this notation for a range based conditional!

Comment thread adagrams/game.py
current_score = score_word(word)

# Update best_word and best_score if conditions are met
if (

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I love that you handled all of the logic as each word gets scored! I also commend you for taking care of all the logic in one long compound conditional! That being said, I would argue that this is one case where nested conditionals will make the code a little more readable. It will also cut down on some of the conditionals you have to parse through!

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