Skip to content
This repository was archived by the owner on Sep 1, 2022. It is now read-only.

Update random_fact selection method #16

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kirillperesh
Copy link

@kirillperesh kirillperesh commented Jan 18, 2021

I suppose, defining a new function (def select_random_fact(fact_arr)) is redundant in this case. And for people who aren't familiar with 'random' module or python in general, it might be much easier to understand this part this way, then via generating random index.
Also, 'select_random_fact()' if left as is, might raise 'IndexError: list index out of range' at some point. 'len(fact_list)+1' should be changed to 'len(fact_list)-1'

I suppose, defining a new function (def select_random_fact(fact_arr)) is redundant in this case. And for people who aren't familiar with 'random' module or python in general, it might be much easier to understand this part this way, then via generating random index.
Also, 'select_random_fact()' if left as is, might raise 'IndexError: list index out of range' at some point. 'len(fact_list)+1' should be changed to 'len(fact_list)-1'
@github-learning-lab github-learning-lab bot had a problem deploying to production January 18, 2021 14:36 Failure
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant