Fishing simulator
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty{ margin-bottom:0;
}
$begingroup$
I'm new to Python, for a school project I created a "fishing simulator". Basically, it is a use of random. I know that my code is repetitive towards the end, but I don't know how to simplify it.
import time
import random
fishing = True
a = b = c = d = e = 0 #define multiple variables as same thing
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print ("Welcome to Lake Tocowaga")
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
time.sleep(1)
name = input("What is your name fisherman?")
answer = input("Would you like to go fishing, " + name + "?")
if answer.lower() == "no":
fishing == False
while fishing == True:
time.sleep(1)
answer = input("Throw out your line, or go home?")
if answer == "go home":
fishing = False
er = float(e / (a + b + c + d))
print("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print("Thanks for playing " + name + "!")
print("You caught:", str(a), "cod, ", str(b), "salmon, ", str(c), "shark, ", str(d), "wildfish. nEfficiency Rate: ", str(er), ".")
else:
t = random.randrange(1, 7)
if t == 1:
a += 1
print("You caught a cod!")
elif t == 2:
b += 1
print("You caught a salmon!")
elif t == 3:
c += 1
print("You caught a shark!")
elif t == 4:
d += 1
print("You caught a wildfish!")
elif t >= 5:
e += 1
print("You caught nothing!")
python beginner python-3.x
New contributor
$endgroup$
add a comment |
$begingroup$
I'm new to Python, for a school project I created a "fishing simulator". Basically, it is a use of random. I know that my code is repetitive towards the end, but I don't know how to simplify it.
import time
import random
fishing = True
a = b = c = d = e = 0 #define multiple variables as same thing
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print ("Welcome to Lake Tocowaga")
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
time.sleep(1)
name = input("What is your name fisherman?")
answer = input("Would you like to go fishing, " + name + "?")
if answer.lower() == "no":
fishing == False
while fishing == True:
time.sleep(1)
answer = input("Throw out your line, or go home?")
if answer == "go home":
fishing = False
er = float(e / (a + b + c + d))
print("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print("Thanks for playing " + name + "!")
print("You caught:", str(a), "cod, ", str(b), "salmon, ", str(c), "shark, ", str(d), "wildfish. nEfficiency Rate: ", str(er), ".")
else:
t = random.randrange(1, 7)
if t == 1:
a += 1
print("You caught a cod!")
elif t == 2:
b += 1
print("You caught a salmon!")
elif t == 3:
c += 1
print("You caught a shark!")
elif t == 4:
d += 1
print("You caught a wildfish!")
elif t >= 5:
e += 1
print("You caught nothing!")
python beginner python-3.x
New contributor
$endgroup$
3
$begingroup$
Why did you tag this question as python-2.x? I'm not convinced that it would work correctly in Python 2.
$endgroup$
– 200_success
2 days ago
add a comment |
$begingroup$
I'm new to Python, for a school project I created a "fishing simulator". Basically, it is a use of random. I know that my code is repetitive towards the end, but I don't know how to simplify it.
import time
import random
fishing = True
a = b = c = d = e = 0 #define multiple variables as same thing
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print ("Welcome to Lake Tocowaga")
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
time.sleep(1)
name = input("What is your name fisherman?")
answer = input("Would you like to go fishing, " + name + "?")
if answer.lower() == "no":
fishing == False
while fishing == True:
time.sleep(1)
answer = input("Throw out your line, or go home?")
if answer == "go home":
fishing = False
er = float(e / (a + b + c + d))
print("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print("Thanks for playing " + name + "!")
print("You caught:", str(a), "cod, ", str(b), "salmon, ", str(c), "shark, ", str(d), "wildfish. nEfficiency Rate: ", str(er), ".")
else:
t = random.randrange(1, 7)
if t == 1:
a += 1
print("You caught a cod!")
elif t == 2:
b += 1
print("You caught a salmon!")
elif t == 3:
c += 1
print("You caught a shark!")
elif t == 4:
d += 1
print("You caught a wildfish!")
elif t >= 5:
e += 1
print("You caught nothing!")
python beginner python-3.x
New contributor
$endgroup$
I'm new to Python, for a school project I created a "fishing simulator". Basically, it is a use of random. I know that my code is repetitive towards the end, but I don't know how to simplify it.
import time
import random
fishing = True
a = b = c = d = e = 0 #define multiple variables as same thing
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print ("Welcome to Lake Tocowaga")
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
time.sleep(1)
name = input("What is your name fisherman?")
answer = input("Would you like to go fishing, " + name + "?")
if answer.lower() == "no":
fishing == False
while fishing == True:
time.sleep(1)
answer = input("Throw out your line, or go home?")
if answer == "go home":
fishing = False
er = float(e / (a + b + c + d))
print("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print("Thanks for playing " + name + "!")
print("You caught:", str(a), "cod, ", str(b), "salmon, ", str(c), "shark, ", str(d), "wildfish. nEfficiency Rate: ", str(er), ".")
else:
t = random.randrange(1, 7)
if t == 1:
a += 1
print("You caught a cod!")
elif t == 2:
b += 1
print("You caught a salmon!")
elif t == 3:
c += 1
print("You caught a shark!")
elif t == 4:
d += 1
print("You caught a wildfish!")
elif t >= 5:
e += 1
print("You caught nothing!")
python beginner python-3.x
python beginner python-3.x
New contributor
New contributor
edited 10 hours ago
Stephen Rauch
3,77061630
3,77061630
New contributor
asked 2 days ago
MattthecommieMattthecommie
1487
1487
New contributor
New contributor
3
$begingroup$
Why did you tag this question as python-2.x? I'm not convinced that it would work correctly in Python 2.
$endgroup$
– 200_success
2 days ago
add a comment |
3
$begingroup$
Why did you tag this question as python-2.x? I'm not convinced that it would work correctly in Python 2.
$endgroup$
– 200_success
2 days ago
3
3
$begingroup$
Why did you tag this question as python-2.x? I'm not convinced that it would work correctly in Python 2.
$endgroup$
– 200_success
2 days ago
$begingroup$
Why did you tag this question as python-2.x? I'm not convinced that it would work correctly in Python 2.
$endgroup$
– 200_success
2 days ago
add a comment |
5 Answers
5
active
oldest
votes
$begingroup$
Welcome to CodeReview. It's never too early to develop good coding habits, and reviewing your code is about the best way to do so.
First, congratulations on writing a clean, straightforward program. While you do have some issues (below), they're not major, and your program seems appropriate for its level.
Now, for the issues ;-)
Use whitespace
Python requires you to use horizontal whitespace. But you should also use vertical whitespace (aka "blank lines") to organize the different parts of your code into paragraphs.
This huge block:
import time
import random
fishing = True
a = b = c = d = e = 0 #define multiple variables as same thing
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print ("Welcome to Lake Tocowaga")
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
time.sleep(1)
name = input("What is your name fisherman?")
answer = input("Would you like to go fishing, " + name + "?")
if answer.lower() == "no":
fishing == False
while fishing == True:
would read better if it were broken up like so:
import time
import random
fishing = True
a = b = c = d = e = 0 #define multiple variables as same thing
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print ("Welcome to Lake Tocowaga")
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
time.sleep(1)
name = input("What is your name fisherman?")
answer = input("Would you like to go fishing, " + name + "?")
if answer.lower() == "no":
fishing == False
while fishing == True:
All I did was add a few blank lines, but I was trying to show that "these things go together" and "these things are in sequence but not related".
Use meaningful names:
Which one of these is the shark?
a = b = c = d = e = 0
I have no idea. But if you named them appropriately:
cod = shark = wildfish = salmon = nothing = 0
I would know for sure!
Use named constants
This line appears three times:
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
It's probably hard to get the right number of tilde characters, unless you are copy/pasting it. And if you're doing that, it's probably a pain. Instead, create a name for the tildes. By convention, constants are spelled in uppercase. (It's not really a constant, but since constants are spelled in upper case, if you name it in upper case you'll know not to modify it.)
H_LINE = "~" * 32
print(H_LINE)
print("Welcome to Lake Tocowaga")
print(H_LINE)
Put last things last
There's a place for everything. And everything should be in its place. The place for printing a summary would be at the bottom.
You had a good idea with your while fishing:
loop. But instead of immediately printing the summary when you respond to the user input, just change the variable and let the loop fail, then print the summary at the bottom. It's more "natural" (and it makes your loops easier to read!).
while fishing == True:
time.sleep(1)
answer = input("Throw out your line, or go home?")
if answer == "go home":
fishing = False
er = float(e / (a + b + c + d))
print("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print("Thanks for playing " + name + "!")
print("You caught:", str(a), "cod, ", str(b), "salmon, ", str(c), "shark, ", str(d), "wildfish. nEfficiency Rate: ", str(er), ".")
else:
...
Becomes:
while fishing == True:
time.sleep(1)
answer = input("Throw out your line, or go home?")
if answer == "go home":
fishing = False
else:
...
er = float(e / (a + b + c + d))
print(H_LINE)
print("Thanks for playing " + name + "!")
print("You caught:", str(a), "cod, ", str(b), "salmon, ", str(c), "shark, ", str(d), "wildfish. nEfficiency Rate: ", str(er), ".")
Let the built-in functions do their job
You are calling functions that you don't need to call. The result of "true" division between integers is a float. You don't need to call float(e / (a + b + c + d))
. And if you did need to call it, you'd be calling it too late!
Likewise, print
knows how to handle integers and floating point numbers. You don't need to print(..., str(a), ...)
when you can just do: print(..., a, ...)
.
$endgroup$
add a comment |
$begingroup$
A few simple things.
a = b = c = d = e = 0
This is bad for a couple reasons:
Those are all nondescript, overly simple names. There's no way to tell what they represent just by looking at them.
You're shoving their declarations/definitions all on one line. This is generally regarded as poor practice. Say I'm looking for where
c
is defined. It's much easier to find it when I can be sure that I'm looking for exactlyc = ...
somewhere. It's harder to find though when it's declared half way through a line.
In both cases, you're sacrificing readability for brevity. Avoid doing this unless you're code golfing. Readability takes precedence over nearly everything else.
fishing = True
is the third line in your file, yet you don't use it until later. Unless it's a constant, it's a good idea to declare variables near where they're first used. When someone's reading your code and wants to see the definition of fishing
, it's more efficient if they only have to look up a line or two instead of needing to scroll to the top of the file.
while fishing == True:
can simply be written as while fishing:
.
You actually have a bug. fishing == False
should be fishing = False
.
if answer.lower() == "no":
could be written to be more "tolerant" (but less exact) by only checking the first letter:
if answer.lower().startswith("n"):
Now input like "nope" will work as well. Whether or not you want this behavior is another story though. If you had other answers that require "n" as the first letter, obviously this would break things.
$endgroup$
$begingroup$
Aboutwhile fishing
: Suppose, say by user input or something,fishing
is a string, thenwhile fishing
would always beTrue
. Isn't it safer to writewhile fishing is True
?
$endgroup$
– niko
20 hours ago
$begingroup$
if answer.lower()[0] == "n":
will break ifanswer
is an empty string. Betterif answer.lower().startswith('n'):
. :-)
$endgroup$
– TrebledJ
19 hours ago
$begingroup$
@nikofishing
is never assigned user input. If it was accidentally, any behavior resulting from that would be incorrect, so I don't think usingis True
would help anything.
$endgroup$
– Carcigenicate
13 hours ago
$begingroup$
@TrebledJ Yes, good call.
$endgroup$
– Carcigenicate
13 hours ago
add a comment |
$begingroup$
First off I think your use case is a nifty way of getting into Python, and it looks like aside from the bugs that others have already pointed out you'll likely soon be unstoppable.
However, instead of simplifying the code I'd suggest modularizing as well as making use of __doc__
strings. It'll make adding features much easier in the future, and if you so choose, allow for making a full application with Kivy
, Blender
, or one of the other many GUI frameworks for Python development. Plus modularizing or abstraction allows for simplifying the intentions/usage.
Some notes before diving-in...
it's probably a good idea to get a snack and drink; I'm a bit verbose and am about to compress some years of knowledge
__bar__
when spoken is "dunder bar" , and the phylum that their classified under are "magic methods"
... okay back on track.
Here's some example code inspired by yours that shows some of what I was going on about in your question's comments...
#!/usr/bin/env python
import time
import random
print_separator = "".join(['_' for _ in range(9)])
__author__ = "S0AndS0"
class Gone_Fishing(dict):
"""
Gone_Fishing is a simple simulation inspired by
[Python - Fishing Simulator](https://codereview.stackexchange.com/q/217357/197446)
## Arguments
- `fishes`, `dict`ionary such as `{'cod': {'amount': 0, 'chances': [1, 2]}}`
- `min_chance`, `int`eger of min number that `random.randint` may generate
- `max_chance`, `int`eger of max number that `random.randint` may generate
"""
def __init__(self, fishes, min_chance = 1, max_chance = 10, **kwargs):
super(Gone_Fishing, self).__init__(**kwargs)
self.update(fishes = fishes,
chances = {'min': min_chance, 'max': max_chance})
@staticmethod
def question(message):
""" Returns response to `message` from user """
return input("{message}? ".format(message = message))
@staticmethod
def keep_fishing(response, expected):
""" Return `bool`ean of if `response` matches `expected` """
if not response or not isinstance(response, str):
return False
return response.lower() == expected
@property
def dump_cooler(self):
"""
Returns `score`, a `dict`ionary similar to `{'cod': 5, 'tire': 2}`,
after printing and reseting _`amount`s_ caught
"""
score = {}
for fish, data in self['fishes'].items():
if data['amount'] > 0:
score.update({fish: data['amount']})
if data['amount'] > 1 and data.get('plural'):
fish = data['plural']
print("{amount} {fish}".format(**{
'fish': fish,
'amount': data['amount']}))
data['amount'] = 0
return score
def catch(self, chance):
""" Returns `None` or name of `fish` caught based on `chance` """
caught =
for fish, data in self['fishes'].items():
if chance in data['chances']:
caught.append(fish)
return caught
def main_loop(self):
"""
Asks questions, adds to _cooler_ anything caught, and prints score when finished
"""
first = True
message = 'Go fishing'
expected = 'yes'
while self.keep_fishing(self.question(message), expected):
time.sleep(1)
if first:
first = False
message = "Keep fishing"
chances = random.randint(self['chances']['min'], self['chances']['max'])
caught = self.catch(chances)
if caught:
for fish in caught:
self['fishes'][fish]['amount'] += 1
fancy_fish = ' '.join(fish.split('_')).title()
print("You caught a {fish}".format(fish = fancy_fish))
else:
print("Nothing was caught this time.")
print("{0}nThanks for playing".format(print_separator))
if True in [x['amount'] > 0 for x in self['fishes'].values()]:
print("You caught")
self.dump_cooler
print(print_separator)
if __name__ == '__main__':
"""
This block of code is not executed during import
and instead is usually run when a file is executed,
eg. `python gone_fishing.py`, making it a good
place for simple unit tests and example usage.
"""
gone_fishing = Gone_Fishing(
fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [5]},
'shark': {'amount': 0, 'chances': [9, 10], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [7], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [10, 15], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [2, 19], 'plural': 'tires'},
},
min_chances = 0,
max_chances = 20,
)
gone_fishing.main_loop()
... okay there's a bit going on up there, so feel free to dissect it's operation by adding breakpoints
or print(something)
lines.
Here's what output of running the above script may look like
# python gone_fishing.py
Go fishing? 'yes'
You caught a Wild Fish
Keep fishing? 'yes'
Nothing was caught this time.
Keep fishing? 'yes'
You caught a Shark
You caught a Old Shoe
Keep fishing? 'yes'
Nothing was caught this time.
# ... trimmed for brevity
Keep fishing? 'no'
_________
Thanks for playing
You caught
2 sharks
1 tire
2 wild_fishes
1 cod
_________
Taking it from the top print_separator = "".join(['_' for _ in range(9)])
is what I like to use when generating strings of repeating characters because it's easy to make something that outputs _-_-_
via "-".join(['_' for _ in range(3)])
.
By defining a class that inherits from the built in dict
ionary class
(that's what the class Gone_Fishing(dict):
line did), I'm being a bit lazy as this allows for dumping all saved states via...
print(gone_fishing)
# -> {'cod': {'amount': 2, 'chances': [1]}, ...}
... and while I'm on the tangent of getting info back out...
print(gone_fishing.main_loop.__doc__)
# Or
# help(gone_fishing.main_loop)
... will print the previously mentioned
__doc__
strings.
... and figuring out where you too can avoid re-inventing the wheel is just something that'll get picked up over time. Personally I choose to view it as expanding one's vocabulary, when I discover some built-in that's been waiting to solve some edge-case.
The __init__
method
absorbs three arguments and re-assigns'em with self.update()
so that other methods that use the self
argument are able to get and/or modify class
saved states; more on that latter.
That bit with **kwargs
stands for key word arguments
which passes things as a bare dict
ionary, the other syntax you may run across is *args
, which passes things as a bare list
of arguments; there be some fanciness that can be done with this syntax that I'll not get into at this point. However, you'll find some examples of passing an unwrapped dictionary, such as to format
via print("{amount} {fish}".format(**{...}))
, which hint hint, is a great way of passing variable parameter names.
This is one of those idiomatic things that you can pick-up with some experimentation (and grokking-out others' code bases); it's super powerful so use it often but be kind to your future self too.
The bit with super(Gone_Fishing, self).__init__(**kwargs)
is what allows the Gone_Fishing
class
to call dict
's __init__
from within it's own __init__
method
... indeed that was a little convoluted so taking a sec to unpack that...
class SomeThing(dict):
def __init__(self, an_argument = None, **kwargs):
super(SomeThing, self).__init__(**kwargs)
self.update({'an_argument': an_argument})
... it's possible to call self.update()
from within SomeThing.___init__
without causing confusion of intent, where as to have SomeThing
still operate as a dict
ionary, eg. assigning something = SomeThing(spam = 'Spam')
without causing errors, one should use super(SomeThing, self).__init__(**kwargs)
to allow Python to preform it's voodoo with figuring out which inheriting class
'll take responsibility for those arguments. Side note, that does mean that one could do class SomeThing(dict, Iterator)
, and have that mean something but I'll not get into that here; kinda already covered that specifically on math stack in regards to graph modeling and prioritization.
The @staticmethod
and other decorators
are ways of denoting a special use method
. In the case of property
s they operate similarly to Object
properties, eg...
class Test_Obj:
pass
o = Test_Obj()
o.foo = 'Foo'
print(o.foo)
# -> Foo
... but can only be gotten not set, which makes'em a great place to stash dynamic or semiprivate properties about an object.
In the case of staticmethod
s, they're not passed a reference to self
so cannot easily access or modify saved states, but they can be more easily used without initializing, eg...
responses =
responses.append(Gone_Fishing.question("Where to"))
print("I heard -> {response}".format(response = responses[-1]))
for _ in range(7):
responses.append(Gone_Fishing.question("... are you sure"))
print("I heard -> {response}".format(response = responses[-1]))
print("Okay... though...")
Note also the various
.format()
usages are to show ways of future prepping (for perhaps usingf strings
in the future), as well as making strings somewhat more explicit.
Generally I use'em to make the intended usage more explicit but that's not to say that you couldn't get lost in the amount of options available just for decorating a method
.
That bit within the main_loop
method
with while self.keep_fishing(self.question(message), expected)
, when unwrapped I think you'll really like, it's returning True
or False
at the top of every iteration based on asking the user a question and comparing their response with what's expected.
And the bit with if True in [x['amount'] > 0 for x in self['fishes'].values()]
is something that masks data using list comprehensions
, I'll advise against getting too fancy with'em, and instead try to utilize'em whenever it doesn't make code less readable. Also don't get to attached to such cleverness because numpy
, pandas
, or one of the many other libraries, will preform similar tasks far faster.
The things happening bellow the if __name__ == '__main__':
, aside from the doc string ...
Side note for those new to Python; sure you could call'em "dunder docs" and those in the know would know what you where saying, but they'd also likely smize at ya too, and saying "dundar doc string" if timed when a listener is drinking could have messy consequences... so "pro-tip", callem "doc strings" to be
super
class
y when talking about Python code ;-)
gone_fishing = Gone_Fishing(fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [2]},
'shark': {'amount': 0, 'chances': [3], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [4], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [5, 6], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [7, 8], 'plural': 'tires'},
})
... and how the above is parsed could take some words to do a full stack trace, but the gist is that chances
is a list
that you could even have overlapping integers, eg. a shark
who had an old_shoe
inside could be...
gone_fishing['fishes']['shark']['chances'].append(5)
... though without adjustments to other values that would make for a very large shoal of soul hungry sharks.
Note from the future; I've made adjustments to the code to enable overlapping values and returning of more than one result; there probably be better ways of doing it but this is also an example of iterative development now.
When you've figured out how plural
is an optional key value pair within a nested dictionary you'll start seeing similar things in other code (at least it's one of those things I've not been unable to unsee), try not to get messy with that trick though, otherwise I think it's self-explanatory as to the intentions of it's usage.
The arguments that I didn't assign, min_chance
and max_chance
, much like the chances
with sharks
could be updated similarly, eg...
gone_fishing['chances']['max'] = 20
... though initializing a new trip would look like...
another_fishing_trip = Gone_Fishing(
fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [5]},
'shark': {'amount': 0, 'chances': [9, 10], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [7], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [10, 15], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [2, 19], 'plural': 'tires'},
},
min_chances = 0,
max_chances = 20,
)
... which serves as an example of something you'd be wise to avoid doing to your own code, swapping words especially isn't going to win any points from a future self or other developers.
There's certainly more room for improvement, eg. having gone_fishing['fishes'][fish_name]['ammount']
subtracted from, while adding to gone_fishing['cooler']
or similar structure; just for a start. But this was all just to expose quick-n-dirty methods of organizing the problem space with Object Oriented Programing.
Hopefully having code with a bit more abstraction shows ya that going with something that looks a bit more complex can allow for simplifying the usage and future feature creep
. Please keep us posted if ya make something more out of your learning project.
New contributor
$endgroup$
4
$begingroup$
Welcome to Code Review! I think you did a grat job on your first answer here. Keep it going!
$endgroup$
– Alex
yesterday
1
$begingroup$
A lot of great advice, but I would like to note that this:print_separator = "".join(['_' for _ in range(9)])
Can be shortened to this:print_separator = '_' * 9
$endgroup$
– shellster
yesterday
1
$begingroup$
print("{amount} {fish}".format(**{'fish': fish, 'amount': data['amount']}))
-- why notprint("{amount} {fish}".format(fish=fish, amount=data['amount']))
?
$endgroup$
– kevinsa5
23 hours ago
1
$begingroup$
@S0AndS0 This is obviously a tiny nitpick, but even then it would probably be better to use one of"--".join('_' * 3)
(limitations: underscore must be single character and builds intermediary string),"--".join('_' for _ in range(3))
,"--".join(itertools.repeat('_', 3))
, depending whether or not you likeitertools
. Constructing an intermediary list in memory here is really not necessary.
$endgroup$
– Izaak van Dongen
9 hours ago
1
$begingroup$
Indeed an intermediatelist
is uncalled for in the example use case, and I'm glad that you've pointed out a plethora of other options. That first one is sweet @Izaak van Dongen! And has some interesting outputs when more than one character is used, eg."--".join('_|' * 3)
->
_--|--_--|--_--|
. Especially when compared to the output of your other suggestion"--".join('_|' for _ in range(3))
->
_|--_|--_|
... definitely something that I'll keep in mind for fancy dividers.
$endgroup$
– S0AndS0
8 hours ago
|
show 3 more comments
$begingroup$
This is another inprovement using a dictionary. Currently all of your data is hardcoded and distributed somewhere in the code. If you wanted to add another fish, you would have to add a variable f
, extend random.randint
(so the chance for nothing does not decrease) and finally add it to the if
conditions and the printing.
That is a lot of work just to add one more fish. Instead I would propose to use a dictionary of possible fishing outcomes and their chance of being caught. You can then use this with random.choices
, which takes a weights
argument detailing the probabilities.
pond = {'cod': 1, 'salmon': 1, 'shark': 1, 'wildfish': 1, 'nothing': 2}
The probabilities are here just relative to each other, random.choices
normalizes them for you. All fish have the same probability and getting nothing has double the probability of any single fish.
Your loop also does not need the fishing
variable at all, just break
it when the user is done fishing.
Whenever you need to count something, using collections.Counter
is probably a good idea. It basically works like a dictionary and has the nice feature that it assumes all elements have a count of zero.
In Python 3.6 a new way to format strings was introduced, the f-string
.
from collections import Counter
from random import choices
from time import sleep
POND = {'cod': 1, 'salmon': 1, 'shark': 1, 'wildfish': 1, 'nothing': 2}
name = input("What is your name fisherman? ")
caught = Counter()
while True:
keep_fishing = input("Throw out your line, or go home? ")
if keep_fishing == "go home":
break
sleep(1)
result = choices(list(POND), weights=POND.values(), k=1)[0]
print(f"You caught: {result}")
caught[result] += 1
print(f"nThanks for playing, {name}!")
print("You caught:")
for fish, n in caught.most_common():
if fish != "nothing":
print(n, fish)
$endgroup$
$begingroup$
For the behavior to be the same as the original code, nothing should have chance 3
$endgroup$
– Vaelus
6 hours ago
$begingroup$
@Vaelusrandrange
is exclusive of the end, so produces vales from 1 to 6 (inclusive), therefore only 5 and 6 producenothing
.
$endgroup$
– Graipher
5 hours ago
$begingroup$
Whoops, you're right. Yet another reason to use your method.
$endgroup$
– Vaelus
3 hours ago
add a comment |
$begingroup$
In addition to the other answers, you can also take advantage of python dictionaries:
a = b = c = d = e = 0
...
else:
t = random.randrange(1, 7)
if t == 1:
a += 1
print("You caught a cod!")
elif t == 2:
b += 1
print("You caught a salmon!")
elif t == 3:
c += 1
print("You caught a shark!")
elif t == 4:
d += 1
print("You caught a wildfish!")
elif t >= 5:
e += 1
print("You caught nothing!")
Becomes:
caught_fish = {
'cod': 0,
'salmon': 0,
'shark': 0,
'wildfish': 0,
'nothing': 0,
}
...
else:
t = random.randrange(1,7)
# clamp 't' to dictionary size
if t > len(caught_fish):
t = len(caught_fish)
# pick a type of fish from the list of keys of 'caught_fish' using index 't'
type_of_fish = list(caught_fish)[t - 1]
# update the dictionary
caught_fish[type_of_fish] += 1
# print what type of fish was caught, or if no fish was caught
article = 'a ' if type_of_fish != 'nothing' else ''
print("You caught {}{}!".format(article, type_of_fish))
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Mattthecommie is a new contributor. Be nice, and check out our Code of Conduct.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f217357%2ffishing-simulator%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
5 Answers
5
active
oldest
votes
5 Answers
5
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
Welcome to CodeReview. It's never too early to develop good coding habits, and reviewing your code is about the best way to do so.
First, congratulations on writing a clean, straightforward program. While you do have some issues (below), they're not major, and your program seems appropriate for its level.
Now, for the issues ;-)
Use whitespace
Python requires you to use horizontal whitespace. But you should also use vertical whitespace (aka "blank lines") to organize the different parts of your code into paragraphs.
This huge block:
import time
import random
fishing = True
a = b = c = d = e = 0 #define multiple variables as same thing
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print ("Welcome to Lake Tocowaga")
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
time.sleep(1)
name = input("What is your name fisherman?")
answer = input("Would you like to go fishing, " + name + "?")
if answer.lower() == "no":
fishing == False
while fishing == True:
would read better if it were broken up like so:
import time
import random
fishing = True
a = b = c = d = e = 0 #define multiple variables as same thing
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print ("Welcome to Lake Tocowaga")
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
time.sleep(1)
name = input("What is your name fisherman?")
answer = input("Would you like to go fishing, " + name + "?")
if answer.lower() == "no":
fishing == False
while fishing == True:
All I did was add a few blank lines, but I was trying to show that "these things go together" and "these things are in sequence but not related".
Use meaningful names:
Which one of these is the shark?
a = b = c = d = e = 0
I have no idea. But if you named them appropriately:
cod = shark = wildfish = salmon = nothing = 0
I would know for sure!
Use named constants
This line appears three times:
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
It's probably hard to get the right number of tilde characters, unless you are copy/pasting it. And if you're doing that, it's probably a pain. Instead, create a name for the tildes. By convention, constants are spelled in uppercase. (It's not really a constant, but since constants are spelled in upper case, if you name it in upper case you'll know not to modify it.)
H_LINE = "~" * 32
print(H_LINE)
print("Welcome to Lake Tocowaga")
print(H_LINE)
Put last things last
There's a place for everything. And everything should be in its place. The place for printing a summary would be at the bottom.
You had a good idea with your while fishing:
loop. But instead of immediately printing the summary when you respond to the user input, just change the variable and let the loop fail, then print the summary at the bottom. It's more "natural" (and it makes your loops easier to read!).
while fishing == True:
time.sleep(1)
answer = input("Throw out your line, or go home?")
if answer == "go home":
fishing = False
er = float(e / (a + b + c + d))
print("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print("Thanks for playing " + name + "!")
print("You caught:", str(a), "cod, ", str(b), "salmon, ", str(c), "shark, ", str(d), "wildfish. nEfficiency Rate: ", str(er), ".")
else:
...
Becomes:
while fishing == True:
time.sleep(1)
answer = input("Throw out your line, or go home?")
if answer == "go home":
fishing = False
else:
...
er = float(e / (a + b + c + d))
print(H_LINE)
print("Thanks for playing " + name + "!")
print("You caught:", str(a), "cod, ", str(b), "salmon, ", str(c), "shark, ", str(d), "wildfish. nEfficiency Rate: ", str(er), ".")
Let the built-in functions do their job
You are calling functions that you don't need to call. The result of "true" division between integers is a float. You don't need to call float(e / (a + b + c + d))
. And if you did need to call it, you'd be calling it too late!
Likewise, print
knows how to handle integers and floating point numbers. You don't need to print(..., str(a), ...)
when you can just do: print(..., a, ...)
.
$endgroup$
add a comment |
$begingroup$
Welcome to CodeReview. It's never too early to develop good coding habits, and reviewing your code is about the best way to do so.
First, congratulations on writing a clean, straightforward program. While you do have some issues (below), they're not major, and your program seems appropriate for its level.
Now, for the issues ;-)
Use whitespace
Python requires you to use horizontal whitespace. But you should also use vertical whitespace (aka "blank lines") to organize the different parts of your code into paragraphs.
This huge block:
import time
import random
fishing = True
a = b = c = d = e = 0 #define multiple variables as same thing
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print ("Welcome to Lake Tocowaga")
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
time.sleep(1)
name = input("What is your name fisherman?")
answer = input("Would you like to go fishing, " + name + "?")
if answer.lower() == "no":
fishing == False
while fishing == True:
would read better if it were broken up like so:
import time
import random
fishing = True
a = b = c = d = e = 0 #define multiple variables as same thing
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print ("Welcome to Lake Tocowaga")
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
time.sleep(1)
name = input("What is your name fisherman?")
answer = input("Would you like to go fishing, " + name + "?")
if answer.lower() == "no":
fishing == False
while fishing == True:
All I did was add a few blank lines, but I was trying to show that "these things go together" and "these things are in sequence but not related".
Use meaningful names:
Which one of these is the shark?
a = b = c = d = e = 0
I have no idea. But if you named them appropriately:
cod = shark = wildfish = salmon = nothing = 0
I would know for sure!
Use named constants
This line appears three times:
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
It's probably hard to get the right number of tilde characters, unless you are copy/pasting it. And if you're doing that, it's probably a pain. Instead, create a name for the tildes. By convention, constants are spelled in uppercase. (It's not really a constant, but since constants are spelled in upper case, if you name it in upper case you'll know not to modify it.)
H_LINE = "~" * 32
print(H_LINE)
print("Welcome to Lake Tocowaga")
print(H_LINE)
Put last things last
There's a place for everything. And everything should be in its place. The place for printing a summary would be at the bottom.
You had a good idea with your while fishing:
loop. But instead of immediately printing the summary when you respond to the user input, just change the variable and let the loop fail, then print the summary at the bottom. It's more "natural" (and it makes your loops easier to read!).
while fishing == True:
time.sleep(1)
answer = input("Throw out your line, or go home?")
if answer == "go home":
fishing = False
er = float(e / (a + b + c + d))
print("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print("Thanks for playing " + name + "!")
print("You caught:", str(a), "cod, ", str(b), "salmon, ", str(c), "shark, ", str(d), "wildfish. nEfficiency Rate: ", str(er), ".")
else:
...
Becomes:
while fishing == True:
time.sleep(1)
answer = input("Throw out your line, or go home?")
if answer == "go home":
fishing = False
else:
...
er = float(e / (a + b + c + d))
print(H_LINE)
print("Thanks for playing " + name + "!")
print("You caught:", str(a), "cod, ", str(b), "salmon, ", str(c), "shark, ", str(d), "wildfish. nEfficiency Rate: ", str(er), ".")
Let the built-in functions do their job
You are calling functions that you don't need to call. The result of "true" division between integers is a float. You don't need to call float(e / (a + b + c + d))
. And if you did need to call it, you'd be calling it too late!
Likewise, print
knows how to handle integers and floating point numbers. You don't need to print(..., str(a), ...)
when you can just do: print(..., a, ...)
.
$endgroup$
add a comment |
$begingroup$
Welcome to CodeReview. It's never too early to develop good coding habits, and reviewing your code is about the best way to do so.
First, congratulations on writing a clean, straightforward program. While you do have some issues (below), they're not major, and your program seems appropriate for its level.
Now, for the issues ;-)
Use whitespace
Python requires you to use horizontal whitespace. But you should also use vertical whitespace (aka "blank lines") to organize the different parts of your code into paragraphs.
This huge block:
import time
import random
fishing = True
a = b = c = d = e = 0 #define multiple variables as same thing
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print ("Welcome to Lake Tocowaga")
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
time.sleep(1)
name = input("What is your name fisherman?")
answer = input("Would you like to go fishing, " + name + "?")
if answer.lower() == "no":
fishing == False
while fishing == True:
would read better if it were broken up like so:
import time
import random
fishing = True
a = b = c = d = e = 0 #define multiple variables as same thing
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print ("Welcome to Lake Tocowaga")
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
time.sleep(1)
name = input("What is your name fisherman?")
answer = input("Would you like to go fishing, " + name + "?")
if answer.lower() == "no":
fishing == False
while fishing == True:
All I did was add a few blank lines, but I was trying to show that "these things go together" and "these things are in sequence but not related".
Use meaningful names:
Which one of these is the shark?
a = b = c = d = e = 0
I have no idea. But if you named them appropriately:
cod = shark = wildfish = salmon = nothing = 0
I would know for sure!
Use named constants
This line appears three times:
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
It's probably hard to get the right number of tilde characters, unless you are copy/pasting it. And if you're doing that, it's probably a pain. Instead, create a name for the tildes. By convention, constants are spelled in uppercase. (It's not really a constant, but since constants are spelled in upper case, if you name it in upper case you'll know not to modify it.)
H_LINE = "~" * 32
print(H_LINE)
print("Welcome to Lake Tocowaga")
print(H_LINE)
Put last things last
There's a place for everything. And everything should be in its place. The place for printing a summary would be at the bottom.
You had a good idea with your while fishing:
loop. But instead of immediately printing the summary when you respond to the user input, just change the variable and let the loop fail, then print the summary at the bottom. It's more "natural" (and it makes your loops easier to read!).
while fishing == True:
time.sleep(1)
answer = input("Throw out your line, or go home?")
if answer == "go home":
fishing = False
er = float(e / (a + b + c + d))
print("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print("Thanks for playing " + name + "!")
print("You caught:", str(a), "cod, ", str(b), "salmon, ", str(c), "shark, ", str(d), "wildfish. nEfficiency Rate: ", str(er), ".")
else:
...
Becomes:
while fishing == True:
time.sleep(1)
answer = input("Throw out your line, or go home?")
if answer == "go home":
fishing = False
else:
...
er = float(e / (a + b + c + d))
print(H_LINE)
print("Thanks for playing " + name + "!")
print("You caught:", str(a), "cod, ", str(b), "salmon, ", str(c), "shark, ", str(d), "wildfish. nEfficiency Rate: ", str(er), ".")
Let the built-in functions do their job
You are calling functions that you don't need to call. The result of "true" division between integers is a float. You don't need to call float(e / (a + b + c + d))
. And if you did need to call it, you'd be calling it too late!
Likewise, print
knows how to handle integers and floating point numbers. You don't need to print(..., str(a), ...)
when you can just do: print(..., a, ...)
.
$endgroup$
Welcome to CodeReview. It's never too early to develop good coding habits, and reviewing your code is about the best way to do so.
First, congratulations on writing a clean, straightforward program. While you do have some issues (below), they're not major, and your program seems appropriate for its level.
Now, for the issues ;-)
Use whitespace
Python requires you to use horizontal whitespace. But you should also use vertical whitespace (aka "blank lines") to organize the different parts of your code into paragraphs.
This huge block:
import time
import random
fishing = True
a = b = c = d = e = 0 #define multiple variables as same thing
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print ("Welcome to Lake Tocowaga")
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
time.sleep(1)
name = input("What is your name fisherman?")
answer = input("Would you like to go fishing, " + name + "?")
if answer.lower() == "no":
fishing == False
while fishing == True:
would read better if it were broken up like so:
import time
import random
fishing = True
a = b = c = d = e = 0 #define multiple variables as same thing
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print ("Welcome to Lake Tocowaga")
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
time.sleep(1)
name = input("What is your name fisherman?")
answer = input("Would you like to go fishing, " + name + "?")
if answer.lower() == "no":
fishing == False
while fishing == True:
All I did was add a few blank lines, but I was trying to show that "these things go together" and "these things are in sequence but not related".
Use meaningful names:
Which one of these is the shark?
a = b = c = d = e = 0
I have no idea. But if you named them appropriately:
cod = shark = wildfish = salmon = nothing = 0
I would know for sure!
Use named constants
This line appears three times:
print ("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
It's probably hard to get the right number of tilde characters, unless you are copy/pasting it. And if you're doing that, it's probably a pain. Instead, create a name for the tildes. By convention, constants are spelled in uppercase. (It's not really a constant, but since constants are spelled in upper case, if you name it in upper case you'll know not to modify it.)
H_LINE = "~" * 32
print(H_LINE)
print("Welcome to Lake Tocowaga")
print(H_LINE)
Put last things last
There's a place for everything. And everything should be in its place. The place for printing a summary would be at the bottom.
You had a good idea with your while fishing:
loop. But instead of immediately printing the summary when you respond to the user input, just change the variable and let the loop fail, then print the summary at the bottom. It's more "natural" (and it makes your loops easier to read!).
while fishing == True:
time.sleep(1)
answer = input("Throw out your line, or go home?")
if answer == "go home":
fishing = False
er = float(e / (a + b + c + d))
print("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
print("Thanks for playing " + name + "!")
print("You caught:", str(a), "cod, ", str(b), "salmon, ", str(c), "shark, ", str(d), "wildfish. nEfficiency Rate: ", str(er), ".")
else:
...
Becomes:
while fishing == True:
time.sleep(1)
answer = input("Throw out your line, or go home?")
if answer == "go home":
fishing = False
else:
...
er = float(e / (a + b + c + d))
print(H_LINE)
print("Thanks for playing " + name + "!")
print("You caught:", str(a), "cod, ", str(b), "salmon, ", str(c), "shark, ", str(d), "wildfish. nEfficiency Rate: ", str(er), ".")
Let the built-in functions do their job
You are calling functions that you don't need to call. The result of "true" division between integers is a float. You don't need to call float(e / (a + b + c + d))
. And if you did need to call it, you'd be calling it too late!
Likewise, print
knows how to handle integers and floating point numbers. You don't need to print(..., str(a), ...)
when you can just do: print(..., a, ...)
.
answered 2 days ago
Austin HastingsAustin Hastings
7,9971237
7,9971237
add a comment |
add a comment |
$begingroup$
A few simple things.
a = b = c = d = e = 0
This is bad for a couple reasons:
Those are all nondescript, overly simple names. There's no way to tell what they represent just by looking at them.
You're shoving their declarations/definitions all on one line. This is generally regarded as poor practice. Say I'm looking for where
c
is defined. It's much easier to find it when I can be sure that I'm looking for exactlyc = ...
somewhere. It's harder to find though when it's declared half way through a line.
In both cases, you're sacrificing readability for brevity. Avoid doing this unless you're code golfing. Readability takes precedence over nearly everything else.
fishing = True
is the third line in your file, yet you don't use it until later. Unless it's a constant, it's a good idea to declare variables near where they're first used. When someone's reading your code and wants to see the definition of fishing
, it's more efficient if they only have to look up a line or two instead of needing to scroll to the top of the file.
while fishing == True:
can simply be written as while fishing:
.
You actually have a bug. fishing == False
should be fishing = False
.
if answer.lower() == "no":
could be written to be more "tolerant" (but less exact) by only checking the first letter:
if answer.lower().startswith("n"):
Now input like "nope" will work as well. Whether or not you want this behavior is another story though. If you had other answers that require "n" as the first letter, obviously this would break things.
$endgroup$
$begingroup$
Aboutwhile fishing
: Suppose, say by user input or something,fishing
is a string, thenwhile fishing
would always beTrue
. Isn't it safer to writewhile fishing is True
?
$endgroup$
– niko
20 hours ago
$begingroup$
if answer.lower()[0] == "n":
will break ifanswer
is an empty string. Betterif answer.lower().startswith('n'):
. :-)
$endgroup$
– TrebledJ
19 hours ago
$begingroup$
@nikofishing
is never assigned user input. If it was accidentally, any behavior resulting from that would be incorrect, so I don't think usingis True
would help anything.
$endgroup$
– Carcigenicate
13 hours ago
$begingroup$
@TrebledJ Yes, good call.
$endgroup$
– Carcigenicate
13 hours ago
add a comment |
$begingroup$
A few simple things.
a = b = c = d = e = 0
This is bad for a couple reasons:
Those are all nondescript, overly simple names. There's no way to tell what they represent just by looking at them.
You're shoving their declarations/definitions all on one line. This is generally regarded as poor practice. Say I'm looking for where
c
is defined. It's much easier to find it when I can be sure that I'm looking for exactlyc = ...
somewhere. It's harder to find though when it's declared half way through a line.
In both cases, you're sacrificing readability for brevity. Avoid doing this unless you're code golfing. Readability takes precedence over nearly everything else.
fishing = True
is the third line in your file, yet you don't use it until later. Unless it's a constant, it's a good idea to declare variables near where they're first used. When someone's reading your code and wants to see the definition of fishing
, it's more efficient if they only have to look up a line or two instead of needing to scroll to the top of the file.
while fishing == True:
can simply be written as while fishing:
.
You actually have a bug. fishing == False
should be fishing = False
.
if answer.lower() == "no":
could be written to be more "tolerant" (but less exact) by only checking the first letter:
if answer.lower().startswith("n"):
Now input like "nope" will work as well. Whether or not you want this behavior is another story though. If you had other answers that require "n" as the first letter, obviously this would break things.
$endgroup$
$begingroup$
Aboutwhile fishing
: Suppose, say by user input or something,fishing
is a string, thenwhile fishing
would always beTrue
. Isn't it safer to writewhile fishing is True
?
$endgroup$
– niko
20 hours ago
$begingroup$
if answer.lower()[0] == "n":
will break ifanswer
is an empty string. Betterif answer.lower().startswith('n'):
. :-)
$endgroup$
– TrebledJ
19 hours ago
$begingroup$
@nikofishing
is never assigned user input. If it was accidentally, any behavior resulting from that would be incorrect, so I don't think usingis True
would help anything.
$endgroup$
– Carcigenicate
13 hours ago
$begingroup$
@TrebledJ Yes, good call.
$endgroup$
– Carcigenicate
13 hours ago
add a comment |
$begingroup$
A few simple things.
a = b = c = d = e = 0
This is bad for a couple reasons:
Those are all nondescript, overly simple names. There's no way to tell what they represent just by looking at them.
You're shoving their declarations/definitions all on one line. This is generally regarded as poor practice. Say I'm looking for where
c
is defined. It's much easier to find it when I can be sure that I'm looking for exactlyc = ...
somewhere. It's harder to find though when it's declared half way through a line.
In both cases, you're sacrificing readability for brevity. Avoid doing this unless you're code golfing. Readability takes precedence over nearly everything else.
fishing = True
is the third line in your file, yet you don't use it until later. Unless it's a constant, it's a good idea to declare variables near where they're first used. When someone's reading your code and wants to see the definition of fishing
, it's more efficient if they only have to look up a line or two instead of needing to scroll to the top of the file.
while fishing == True:
can simply be written as while fishing:
.
You actually have a bug. fishing == False
should be fishing = False
.
if answer.lower() == "no":
could be written to be more "tolerant" (but less exact) by only checking the first letter:
if answer.lower().startswith("n"):
Now input like "nope" will work as well. Whether or not you want this behavior is another story though. If you had other answers that require "n" as the first letter, obviously this would break things.
$endgroup$
A few simple things.
a = b = c = d = e = 0
This is bad for a couple reasons:
Those are all nondescript, overly simple names. There's no way to tell what they represent just by looking at them.
You're shoving their declarations/definitions all on one line. This is generally regarded as poor practice. Say I'm looking for where
c
is defined. It's much easier to find it when I can be sure that I'm looking for exactlyc = ...
somewhere. It's harder to find though when it's declared half way through a line.
In both cases, you're sacrificing readability for brevity. Avoid doing this unless you're code golfing. Readability takes precedence over nearly everything else.
fishing = True
is the third line in your file, yet you don't use it until later. Unless it's a constant, it's a good idea to declare variables near where they're first used. When someone's reading your code and wants to see the definition of fishing
, it's more efficient if they only have to look up a line or two instead of needing to scroll to the top of the file.
while fishing == True:
can simply be written as while fishing:
.
You actually have a bug. fishing == False
should be fishing = False
.
if answer.lower() == "no":
could be written to be more "tolerant" (but less exact) by only checking the first letter:
if answer.lower().startswith("n"):
Now input like "nope" will work as well. Whether or not you want this behavior is another story though. If you had other answers that require "n" as the first letter, obviously this would break things.
edited 13 hours ago
answered 2 days ago
CarcigenicateCarcigenicate
4,30911635
4,30911635
$begingroup$
Aboutwhile fishing
: Suppose, say by user input or something,fishing
is a string, thenwhile fishing
would always beTrue
. Isn't it safer to writewhile fishing is True
?
$endgroup$
– niko
20 hours ago
$begingroup$
if answer.lower()[0] == "n":
will break ifanswer
is an empty string. Betterif answer.lower().startswith('n'):
. :-)
$endgroup$
– TrebledJ
19 hours ago
$begingroup$
@nikofishing
is never assigned user input. If it was accidentally, any behavior resulting from that would be incorrect, so I don't think usingis True
would help anything.
$endgroup$
– Carcigenicate
13 hours ago
$begingroup$
@TrebledJ Yes, good call.
$endgroup$
– Carcigenicate
13 hours ago
add a comment |
$begingroup$
Aboutwhile fishing
: Suppose, say by user input or something,fishing
is a string, thenwhile fishing
would always beTrue
. Isn't it safer to writewhile fishing is True
?
$endgroup$
– niko
20 hours ago
$begingroup$
if answer.lower()[0] == "n":
will break ifanswer
is an empty string. Betterif answer.lower().startswith('n'):
. :-)
$endgroup$
– TrebledJ
19 hours ago
$begingroup$
@nikofishing
is never assigned user input. If it was accidentally, any behavior resulting from that would be incorrect, so I don't think usingis True
would help anything.
$endgroup$
– Carcigenicate
13 hours ago
$begingroup$
@TrebledJ Yes, good call.
$endgroup$
– Carcigenicate
13 hours ago
$begingroup$
About
while fishing
: Suppose, say by user input or something, fishing
is a string, then while fishing
would always be True
. Isn't it safer to write while fishing is True
?$endgroup$
– niko
20 hours ago
$begingroup$
About
while fishing
: Suppose, say by user input or something, fishing
is a string, then while fishing
would always be True
. Isn't it safer to write while fishing is True
?$endgroup$
– niko
20 hours ago
$begingroup$
if answer.lower()[0] == "n":
will break if answer
is an empty string. Better if answer.lower().startswith('n'):
. :-)$endgroup$
– TrebledJ
19 hours ago
$begingroup$
if answer.lower()[0] == "n":
will break if answer
is an empty string. Better if answer.lower().startswith('n'):
. :-)$endgroup$
– TrebledJ
19 hours ago
$begingroup$
@niko
fishing
is never assigned user input. If it was accidentally, any behavior resulting from that would be incorrect, so I don't think using is True
would help anything.$endgroup$
– Carcigenicate
13 hours ago
$begingroup$
@niko
fishing
is never assigned user input. If it was accidentally, any behavior resulting from that would be incorrect, so I don't think using is True
would help anything.$endgroup$
– Carcigenicate
13 hours ago
$begingroup$
@TrebledJ Yes, good call.
$endgroup$
– Carcigenicate
13 hours ago
$begingroup$
@TrebledJ Yes, good call.
$endgroup$
– Carcigenicate
13 hours ago
add a comment |
$begingroup$
First off I think your use case is a nifty way of getting into Python, and it looks like aside from the bugs that others have already pointed out you'll likely soon be unstoppable.
However, instead of simplifying the code I'd suggest modularizing as well as making use of __doc__
strings. It'll make adding features much easier in the future, and if you so choose, allow for making a full application with Kivy
, Blender
, or one of the other many GUI frameworks for Python development. Plus modularizing or abstraction allows for simplifying the intentions/usage.
Some notes before diving-in...
it's probably a good idea to get a snack and drink; I'm a bit verbose and am about to compress some years of knowledge
__bar__
when spoken is "dunder bar" , and the phylum that their classified under are "magic methods"
... okay back on track.
Here's some example code inspired by yours that shows some of what I was going on about in your question's comments...
#!/usr/bin/env python
import time
import random
print_separator = "".join(['_' for _ in range(9)])
__author__ = "S0AndS0"
class Gone_Fishing(dict):
"""
Gone_Fishing is a simple simulation inspired by
[Python - Fishing Simulator](https://codereview.stackexchange.com/q/217357/197446)
## Arguments
- `fishes`, `dict`ionary such as `{'cod': {'amount': 0, 'chances': [1, 2]}}`
- `min_chance`, `int`eger of min number that `random.randint` may generate
- `max_chance`, `int`eger of max number that `random.randint` may generate
"""
def __init__(self, fishes, min_chance = 1, max_chance = 10, **kwargs):
super(Gone_Fishing, self).__init__(**kwargs)
self.update(fishes = fishes,
chances = {'min': min_chance, 'max': max_chance})
@staticmethod
def question(message):
""" Returns response to `message` from user """
return input("{message}? ".format(message = message))
@staticmethod
def keep_fishing(response, expected):
""" Return `bool`ean of if `response` matches `expected` """
if not response or not isinstance(response, str):
return False
return response.lower() == expected
@property
def dump_cooler(self):
"""
Returns `score`, a `dict`ionary similar to `{'cod': 5, 'tire': 2}`,
after printing and reseting _`amount`s_ caught
"""
score = {}
for fish, data in self['fishes'].items():
if data['amount'] > 0:
score.update({fish: data['amount']})
if data['amount'] > 1 and data.get('plural'):
fish = data['plural']
print("{amount} {fish}".format(**{
'fish': fish,
'amount': data['amount']}))
data['amount'] = 0
return score
def catch(self, chance):
""" Returns `None` or name of `fish` caught based on `chance` """
caught =
for fish, data in self['fishes'].items():
if chance in data['chances']:
caught.append(fish)
return caught
def main_loop(self):
"""
Asks questions, adds to _cooler_ anything caught, and prints score when finished
"""
first = True
message = 'Go fishing'
expected = 'yes'
while self.keep_fishing(self.question(message), expected):
time.sleep(1)
if first:
first = False
message = "Keep fishing"
chances = random.randint(self['chances']['min'], self['chances']['max'])
caught = self.catch(chances)
if caught:
for fish in caught:
self['fishes'][fish]['amount'] += 1
fancy_fish = ' '.join(fish.split('_')).title()
print("You caught a {fish}".format(fish = fancy_fish))
else:
print("Nothing was caught this time.")
print("{0}nThanks for playing".format(print_separator))
if True in [x['amount'] > 0 for x in self['fishes'].values()]:
print("You caught")
self.dump_cooler
print(print_separator)
if __name__ == '__main__':
"""
This block of code is not executed during import
and instead is usually run when a file is executed,
eg. `python gone_fishing.py`, making it a good
place for simple unit tests and example usage.
"""
gone_fishing = Gone_Fishing(
fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [5]},
'shark': {'amount': 0, 'chances': [9, 10], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [7], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [10, 15], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [2, 19], 'plural': 'tires'},
},
min_chances = 0,
max_chances = 20,
)
gone_fishing.main_loop()
... okay there's a bit going on up there, so feel free to dissect it's operation by adding breakpoints
or print(something)
lines.
Here's what output of running the above script may look like
# python gone_fishing.py
Go fishing? 'yes'
You caught a Wild Fish
Keep fishing? 'yes'
Nothing was caught this time.
Keep fishing? 'yes'
You caught a Shark
You caught a Old Shoe
Keep fishing? 'yes'
Nothing was caught this time.
# ... trimmed for brevity
Keep fishing? 'no'
_________
Thanks for playing
You caught
2 sharks
1 tire
2 wild_fishes
1 cod
_________
Taking it from the top print_separator = "".join(['_' for _ in range(9)])
is what I like to use when generating strings of repeating characters because it's easy to make something that outputs _-_-_
via "-".join(['_' for _ in range(3)])
.
By defining a class that inherits from the built in dict
ionary class
(that's what the class Gone_Fishing(dict):
line did), I'm being a bit lazy as this allows for dumping all saved states via...
print(gone_fishing)
# -> {'cod': {'amount': 2, 'chances': [1]}, ...}
... and while I'm on the tangent of getting info back out...
print(gone_fishing.main_loop.__doc__)
# Or
# help(gone_fishing.main_loop)
... will print the previously mentioned
__doc__
strings.
... and figuring out where you too can avoid re-inventing the wheel is just something that'll get picked up over time. Personally I choose to view it as expanding one's vocabulary, when I discover some built-in that's been waiting to solve some edge-case.
The __init__
method
absorbs three arguments and re-assigns'em with self.update()
so that other methods that use the self
argument are able to get and/or modify class
saved states; more on that latter.
That bit with **kwargs
stands for key word arguments
which passes things as a bare dict
ionary, the other syntax you may run across is *args
, which passes things as a bare list
of arguments; there be some fanciness that can be done with this syntax that I'll not get into at this point. However, you'll find some examples of passing an unwrapped dictionary, such as to format
via print("{amount} {fish}".format(**{...}))
, which hint hint, is a great way of passing variable parameter names.
This is one of those idiomatic things that you can pick-up with some experimentation (and grokking-out others' code bases); it's super powerful so use it often but be kind to your future self too.
The bit with super(Gone_Fishing, self).__init__(**kwargs)
is what allows the Gone_Fishing
class
to call dict
's __init__
from within it's own __init__
method
... indeed that was a little convoluted so taking a sec to unpack that...
class SomeThing(dict):
def __init__(self, an_argument = None, **kwargs):
super(SomeThing, self).__init__(**kwargs)
self.update({'an_argument': an_argument})
... it's possible to call self.update()
from within SomeThing.___init__
without causing confusion of intent, where as to have SomeThing
still operate as a dict
ionary, eg. assigning something = SomeThing(spam = 'Spam')
without causing errors, one should use super(SomeThing, self).__init__(**kwargs)
to allow Python to preform it's voodoo with figuring out which inheriting class
'll take responsibility for those arguments. Side note, that does mean that one could do class SomeThing(dict, Iterator)
, and have that mean something but I'll not get into that here; kinda already covered that specifically on math stack in regards to graph modeling and prioritization.
The @staticmethod
and other decorators
are ways of denoting a special use method
. In the case of property
s they operate similarly to Object
properties, eg...
class Test_Obj:
pass
o = Test_Obj()
o.foo = 'Foo'
print(o.foo)
# -> Foo
... but can only be gotten not set, which makes'em a great place to stash dynamic or semiprivate properties about an object.
In the case of staticmethod
s, they're not passed a reference to self
so cannot easily access or modify saved states, but they can be more easily used without initializing, eg...
responses =
responses.append(Gone_Fishing.question("Where to"))
print("I heard -> {response}".format(response = responses[-1]))
for _ in range(7):
responses.append(Gone_Fishing.question("... are you sure"))
print("I heard -> {response}".format(response = responses[-1]))
print("Okay... though...")
Note also the various
.format()
usages are to show ways of future prepping (for perhaps usingf strings
in the future), as well as making strings somewhat more explicit.
Generally I use'em to make the intended usage more explicit but that's not to say that you couldn't get lost in the amount of options available just for decorating a method
.
That bit within the main_loop
method
with while self.keep_fishing(self.question(message), expected)
, when unwrapped I think you'll really like, it's returning True
or False
at the top of every iteration based on asking the user a question and comparing their response with what's expected.
And the bit with if True in [x['amount'] > 0 for x in self['fishes'].values()]
is something that masks data using list comprehensions
, I'll advise against getting too fancy with'em, and instead try to utilize'em whenever it doesn't make code less readable. Also don't get to attached to such cleverness because numpy
, pandas
, or one of the many other libraries, will preform similar tasks far faster.
The things happening bellow the if __name__ == '__main__':
, aside from the doc string ...
Side note for those new to Python; sure you could call'em "dunder docs" and those in the know would know what you where saying, but they'd also likely smize at ya too, and saying "dundar doc string" if timed when a listener is drinking could have messy consequences... so "pro-tip", callem "doc strings" to be
super
class
y when talking about Python code ;-)
gone_fishing = Gone_Fishing(fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [2]},
'shark': {'amount': 0, 'chances': [3], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [4], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [5, 6], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [7, 8], 'plural': 'tires'},
})
... and how the above is parsed could take some words to do a full stack trace, but the gist is that chances
is a list
that you could even have overlapping integers, eg. a shark
who had an old_shoe
inside could be...
gone_fishing['fishes']['shark']['chances'].append(5)
... though without adjustments to other values that would make for a very large shoal of soul hungry sharks.
Note from the future; I've made adjustments to the code to enable overlapping values and returning of more than one result; there probably be better ways of doing it but this is also an example of iterative development now.
When you've figured out how plural
is an optional key value pair within a nested dictionary you'll start seeing similar things in other code (at least it's one of those things I've not been unable to unsee), try not to get messy with that trick though, otherwise I think it's self-explanatory as to the intentions of it's usage.
The arguments that I didn't assign, min_chance
and max_chance
, much like the chances
with sharks
could be updated similarly, eg...
gone_fishing['chances']['max'] = 20
... though initializing a new trip would look like...
another_fishing_trip = Gone_Fishing(
fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [5]},
'shark': {'amount': 0, 'chances': [9, 10], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [7], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [10, 15], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [2, 19], 'plural': 'tires'},
},
min_chances = 0,
max_chances = 20,
)
... which serves as an example of something you'd be wise to avoid doing to your own code, swapping words especially isn't going to win any points from a future self or other developers.
There's certainly more room for improvement, eg. having gone_fishing['fishes'][fish_name]['ammount']
subtracted from, while adding to gone_fishing['cooler']
or similar structure; just for a start. But this was all just to expose quick-n-dirty methods of organizing the problem space with Object Oriented Programing.
Hopefully having code with a bit more abstraction shows ya that going with something that looks a bit more complex can allow for simplifying the usage and future feature creep
. Please keep us posted if ya make something more out of your learning project.
New contributor
$endgroup$
4
$begingroup$
Welcome to Code Review! I think you did a grat job on your first answer here. Keep it going!
$endgroup$
– Alex
yesterday
1
$begingroup$
A lot of great advice, but I would like to note that this:print_separator = "".join(['_' for _ in range(9)])
Can be shortened to this:print_separator = '_' * 9
$endgroup$
– shellster
yesterday
1
$begingroup$
print("{amount} {fish}".format(**{'fish': fish, 'amount': data['amount']}))
-- why notprint("{amount} {fish}".format(fish=fish, amount=data['amount']))
?
$endgroup$
– kevinsa5
23 hours ago
1
$begingroup$
@S0AndS0 This is obviously a tiny nitpick, but even then it would probably be better to use one of"--".join('_' * 3)
(limitations: underscore must be single character and builds intermediary string),"--".join('_' for _ in range(3))
,"--".join(itertools.repeat('_', 3))
, depending whether or not you likeitertools
. Constructing an intermediary list in memory here is really not necessary.
$endgroup$
– Izaak van Dongen
9 hours ago
1
$begingroup$
Indeed an intermediatelist
is uncalled for in the example use case, and I'm glad that you've pointed out a plethora of other options. That first one is sweet @Izaak van Dongen! And has some interesting outputs when more than one character is used, eg."--".join('_|' * 3)
->
_--|--_--|--_--|
. Especially when compared to the output of your other suggestion"--".join('_|' for _ in range(3))
->
_|--_|--_|
... definitely something that I'll keep in mind for fancy dividers.
$endgroup$
– S0AndS0
8 hours ago
|
show 3 more comments
$begingroup$
First off I think your use case is a nifty way of getting into Python, and it looks like aside from the bugs that others have already pointed out you'll likely soon be unstoppable.
However, instead of simplifying the code I'd suggest modularizing as well as making use of __doc__
strings. It'll make adding features much easier in the future, and if you so choose, allow for making a full application with Kivy
, Blender
, or one of the other many GUI frameworks for Python development. Plus modularizing or abstraction allows for simplifying the intentions/usage.
Some notes before diving-in...
it's probably a good idea to get a snack and drink; I'm a bit verbose and am about to compress some years of knowledge
__bar__
when spoken is "dunder bar" , and the phylum that their classified under are "magic methods"
... okay back on track.
Here's some example code inspired by yours that shows some of what I was going on about in your question's comments...
#!/usr/bin/env python
import time
import random
print_separator = "".join(['_' for _ in range(9)])
__author__ = "S0AndS0"
class Gone_Fishing(dict):
"""
Gone_Fishing is a simple simulation inspired by
[Python - Fishing Simulator](https://codereview.stackexchange.com/q/217357/197446)
## Arguments
- `fishes`, `dict`ionary such as `{'cod': {'amount': 0, 'chances': [1, 2]}}`
- `min_chance`, `int`eger of min number that `random.randint` may generate
- `max_chance`, `int`eger of max number that `random.randint` may generate
"""
def __init__(self, fishes, min_chance = 1, max_chance = 10, **kwargs):
super(Gone_Fishing, self).__init__(**kwargs)
self.update(fishes = fishes,
chances = {'min': min_chance, 'max': max_chance})
@staticmethod
def question(message):
""" Returns response to `message` from user """
return input("{message}? ".format(message = message))
@staticmethod
def keep_fishing(response, expected):
""" Return `bool`ean of if `response` matches `expected` """
if not response or not isinstance(response, str):
return False
return response.lower() == expected
@property
def dump_cooler(self):
"""
Returns `score`, a `dict`ionary similar to `{'cod': 5, 'tire': 2}`,
after printing and reseting _`amount`s_ caught
"""
score = {}
for fish, data in self['fishes'].items():
if data['amount'] > 0:
score.update({fish: data['amount']})
if data['amount'] > 1 and data.get('plural'):
fish = data['plural']
print("{amount} {fish}".format(**{
'fish': fish,
'amount': data['amount']}))
data['amount'] = 0
return score
def catch(self, chance):
""" Returns `None` or name of `fish` caught based on `chance` """
caught =
for fish, data in self['fishes'].items():
if chance in data['chances']:
caught.append(fish)
return caught
def main_loop(self):
"""
Asks questions, adds to _cooler_ anything caught, and prints score when finished
"""
first = True
message = 'Go fishing'
expected = 'yes'
while self.keep_fishing(self.question(message), expected):
time.sleep(1)
if first:
first = False
message = "Keep fishing"
chances = random.randint(self['chances']['min'], self['chances']['max'])
caught = self.catch(chances)
if caught:
for fish in caught:
self['fishes'][fish]['amount'] += 1
fancy_fish = ' '.join(fish.split('_')).title()
print("You caught a {fish}".format(fish = fancy_fish))
else:
print("Nothing was caught this time.")
print("{0}nThanks for playing".format(print_separator))
if True in [x['amount'] > 0 for x in self['fishes'].values()]:
print("You caught")
self.dump_cooler
print(print_separator)
if __name__ == '__main__':
"""
This block of code is not executed during import
and instead is usually run when a file is executed,
eg. `python gone_fishing.py`, making it a good
place for simple unit tests and example usage.
"""
gone_fishing = Gone_Fishing(
fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [5]},
'shark': {'amount': 0, 'chances': [9, 10], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [7], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [10, 15], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [2, 19], 'plural': 'tires'},
},
min_chances = 0,
max_chances = 20,
)
gone_fishing.main_loop()
... okay there's a bit going on up there, so feel free to dissect it's operation by adding breakpoints
or print(something)
lines.
Here's what output of running the above script may look like
# python gone_fishing.py
Go fishing? 'yes'
You caught a Wild Fish
Keep fishing? 'yes'
Nothing was caught this time.
Keep fishing? 'yes'
You caught a Shark
You caught a Old Shoe
Keep fishing? 'yes'
Nothing was caught this time.
# ... trimmed for brevity
Keep fishing? 'no'
_________
Thanks for playing
You caught
2 sharks
1 tire
2 wild_fishes
1 cod
_________
Taking it from the top print_separator = "".join(['_' for _ in range(9)])
is what I like to use when generating strings of repeating characters because it's easy to make something that outputs _-_-_
via "-".join(['_' for _ in range(3)])
.
By defining a class that inherits from the built in dict
ionary class
(that's what the class Gone_Fishing(dict):
line did), I'm being a bit lazy as this allows for dumping all saved states via...
print(gone_fishing)
# -> {'cod': {'amount': 2, 'chances': [1]}, ...}
... and while I'm on the tangent of getting info back out...
print(gone_fishing.main_loop.__doc__)
# Or
# help(gone_fishing.main_loop)
... will print the previously mentioned
__doc__
strings.
... and figuring out where you too can avoid re-inventing the wheel is just something that'll get picked up over time. Personally I choose to view it as expanding one's vocabulary, when I discover some built-in that's been waiting to solve some edge-case.
The __init__
method
absorbs three arguments and re-assigns'em with self.update()
so that other methods that use the self
argument are able to get and/or modify class
saved states; more on that latter.
That bit with **kwargs
stands for key word arguments
which passes things as a bare dict
ionary, the other syntax you may run across is *args
, which passes things as a bare list
of arguments; there be some fanciness that can be done with this syntax that I'll not get into at this point. However, you'll find some examples of passing an unwrapped dictionary, such as to format
via print("{amount} {fish}".format(**{...}))
, which hint hint, is a great way of passing variable parameter names.
This is one of those idiomatic things that you can pick-up with some experimentation (and grokking-out others' code bases); it's super powerful so use it often but be kind to your future self too.
The bit with super(Gone_Fishing, self).__init__(**kwargs)
is what allows the Gone_Fishing
class
to call dict
's __init__
from within it's own __init__
method
... indeed that was a little convoluted so taking a sec to unpack that...
class SomeThing(dict):
def __init__(self, an_argument = None, **kwargs):
super(SomeThing, self).__init__(**kwargs)
self.update({'an_argument': an_argument})
... it's possible to call self.update()
from within SomeThing.___init__
without causing confusion of intent, where as to have SomeThing
still operate as a dict
ionary, eg. assigning something = SomeThing(spam = 'Spam')
without causing errors, one should use super(SomeThing, self).__init__(**kwargs)
to allow Python to preform it's voodoo with figuring out which inheriting class
'll take responsibility for those arguments. Side note, that does mean that one could do class SomeThing(dict, Iterator)
, and have that mean something but I'll not get into that here; kinda already covered that specifically on math stack in regards to graph modeling and prioritization.
The @staticmethod
and other decorators
are ways of denoting a special use method
. In the case of property
s they operate similarly to Object
properties, eg...
class Test_Obj:
pass
o = Test_Obj()
o.foo = 'Foo'
print(o.foo)
# -> Foo
... but can only be gotten not set, which makes'em a great place to stash dynamic or semiprivate properties about an object.
In the case of staticmethod
s, they're not passed a reference to self
so cannot easily access or modify saved states, but they can be more easily used without initializing, eg...
responses =
responses.append(Gone_Fishing.question("Where to"))
print("I heard -> {response}".format(response = responses[-1]))
for _ in range(7):
responses.append(Gone_Fishing.question("... are you sure"))
print("I heard -> {response}".format(response = responses[-1]))
print("Okay... though...")
Note also the various
.format()
usages are to show ways of future prepping (for perhaps usingf strings
in the future), as well as making strings somewhat more explicit.
Generally I use'em to make the intended usage more explicit but that's not to say that you couldn't get lost in the amount of options available just for decorating a method
.
That bit within the main_loop
method
with while self.keep_fishing(self.question(message), expected)
, when unwrapped I think you'll really like, it's returning True
or False
at the top of every iteration based on asking the user a question and comparing their response with what's expected.
And the bit with if True in [x['amount'] > 0 for x in self['fishes'].values()]
is something that masks data using list comprehensions
, I'll advise against getting too fancy with'em, and instead try to utilize'em whenever it doesn't make code less readable. Also don't get to attached to such cleverness because numpy
, pandas
, or one of the many other libraries, will preform similar tasks far faster.
The things happening bellow the if __name__ == '__main__':
, aside from the doc string ...
Side note for those new to Python; sure you could call'em "dunder docs" and those in the know would know what you where saying, but they'd also likely smize at ya too, and saying "dundar doc string" if timed when a listener is drinking could have messy consequences... so "pro-tip", callem "doc strings" to be
super
class
y when talking about Python code ;-)
gone_fishing = Gone_Fishing(fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [2]},
'shark': {'amount': 0, 'chances': [3], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [4], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [5, 6], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [7, 8], 'plural': 'tires'},
})
... and how the above is parsed could take some words to do a full stack trace, but the gist is that chances
is a list
that you could even have overlapping integers, eg. a shark
who had an old_shoe
inside could be...
gone_fishing['fishes']['shark']['chances'].append(5)
... though without adjustments to other values that would make for a very large shoal of soul hungry sharks.
Note from the future; I've made adjustments to the code to enable overlapping values and returning of more than one result; there probably be better ways of doing it but this is also an example of iterative development now.
When you've figured out how plural
is an optional key value pair within a nested dictionary you'll start seeing similar things in other code (at least it's one of those things I've not been unable to unsee), try not to get messy with that trick though, otherwise I think it's self-explanatory as to the intentions of it's usage.
The arguments that I didn't assign, min_chance
and max_chance
, much like the chances
with sharks
could be updated similarly, eg...
gone_fishing['chances']['max'] = 20
... though initializing a new trip would look like...
another_fishing_trip = Gone_Fishing(
fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [5]},
'shark': {'amount': 0, 'chances': [9, 10], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [7], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [10, 15], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [2, 19], 'plural': 'tires'},
},
min_chances = 0,
max_chances = 20,
)
... which serves as an example of something you'd be wise to avoid doing to your own code, swapping words especially isn't going to win any points from a future self or other developers.
There's certainly more room for improvement, eg. having gone_fishing['fishes'][fish_name]['ammount']
subtracted from, while adding to gone_fishing['cooler']
or similar structure; just for a start. But this was all just to expose quick-n-dirty methods of organizing the problem space with Object Oriented Programing.
Hopefully having code with a bit more abstraction shows ya that going with something that looks a bit more complex can allow for simplifying the usage and future feature creep
. Please keep us posted if ya make something more out of your learning project.
New contributor
$endgroup$
4
$begingroup$
Welcome to Code Review! I think you did a grat job on your first answer here. Keep it going!
$endgroup$
– Alex
yesterday
1
$begingroup$
A lot of great advice, but I would like to note that this:print_separator = "".join(['_' for _ in range(9)])
Can be shortened to this:print_separator = '_' * 9
$endgroup$
– shellster
yesterday
1
$begingroup$
print("{amount} {fish}".format(**{'fish': fish, 'amount': data['amount']}))
-- why notprint("{amount} {fish}".format(fish=fish, amount=data['amount']))
?
$endgroup$
– kevinsa5
23 hours ago
1
$begingroup$
@S0AndS0 This is obviously a tiny nitpick, but even then it would probably be better to use one of"--".join('_' * 3)
(limitations: underscore must be single character and builds intermediary string),"--".join('_' for _ in range(3))
,"--".join(itertools.repeat('_', 3))
, depending whether or not you likeitertools
. Constructing an intermediary list in memory here is really not necessary.
$endgroup$
– Izaak van Dongen
9 hours ago
1
$begingroup$
Indeed an intermediatelist
is uncalled for in the example use case, and I'm glad that you've pointed out a plethora of other options. That first one is sweet @Izaak van Dongen! And has some interesting outputs when more than one character is used, eg."--".join('_|' * 3)
->
_--|--_--|--_--|
. Especially when compared to the output of your other suggestion"--".join('_|' for _ in range(3))
->
_|--_|--_|
... definitely something that I'll keep in mind for fancy dividers.
$endgroup$
– S0AndS0
8 hours ago
|
show 3 more comments
$begingroup$
First off I think your use case is a nifty way of getting into Python, and it looks like aside from the bugs that others have already pointed out you'll likely soon be unstoppable.
However, instead of simplifying the code I'd suggest modularizing as well as making use of __doc__
strings. It'll make adding features much easier in the future, and if you so choose, allow for making a full application with Kivy
, Blender
, or one of the other many GUI frameworks for Python development. Plus modularizing or abstraction allows for simplifying the intentions/usage.
Some notes before diving-in...
it's probably a good idea to get a snack and drink; I'm a bit verbose and am about to compress some years of knowledge
__bar__
when spoken is "dunder bar" , and the phylum that their classified under are "magic methods"
... okay back on track.
Here's some example code inspired by yours that shows some of what I was going on about in your question's comments...
#!/usr/bin/env python
import time
import random
print_separator = "".join(['_' for _ in range(9)])
__author__ = "S0AndS0"
class Gone_Fishing(dict):
"""
Gone_Fishing is a simple simulation inspired by
[Python - Fishing Simulator](https://codereview.stackexchange.com/q/217357/197446)
## Arguments
- `fishes`, `dict`ionary such as `{'cod': {'amount': 0, 'chances': [1, 2]}}`
- `min_chance`, `int`eger of min number that `random.randint` may generate
- `max_chance`, `int`eger of max number that `random.randint` may generate
"""
def __init__(self, fishes, min_chance = 1, max_chance = 10, **kwargs):
super(Gone_Fishing, self).__init__(**kwargs)
self.update(fishes = fishes,
chances = {'min': min_chance, 'max': max_chance})
@staticmethod
def question(message):
""" Returns response to `message` from user """
return input("{message}? ".format(message = message))
@staticmethod
def keep_fishing(response, expected):
""" Return `bool`ean of if `response` matches `expected` """
if not response or not isinstance(response, str):
return False
return response.lower() == expected
@property
def dump_cooler(self):
"""
Returns `score`, a `dict`ionary similar to `{'cod': 5, 'tire': 2}`,
after printing and reseting _`amount`s_ caught
"""
score = {}
for fish, data in self['fishes'].items():
if data['amount'] > 0:
score.update({fish: data['amount']})
if data['amount'] > 1 and data.get('plural'):
fish = data['plural']
print("{amount} {fish}".format(**{
'fish': fish,
'amount': data['amount']}))
data['amount'] = 0
return score
def catch(self, chance):
""" Returns `None` or name of `fish` caught based on `chance` """
caught =
for fish, data in self['fishes'].items():
if chance in data['chances']:
caught.append(fish)
return caught
def main_loop(self):
"""
Asks questions, adds to _cooler_ anything caught, and prints score when finished
"""
first = True
message = 'Go fishing'
expected = 'yes'
while self.keep_fishing(self.question(message), expected):
time.sleep(1)
if first:
first = False
message = "Keep fishing"
chances = random.randint(self['chances']['min'], self['chances']['max'])
caught = self.catch(chances)
if caught:
for fish in caught:
self['fishes'][fish]['amount'] += 1
fancy_fish = ' '.join(fish.split('_')).title()
print("You caught a {fish}".format(fish = fancy_fish))
else:
print("Nothing was caught this time.")
print("{0}nThanks for playing".format(print_separator))
if True in [x['amount'] > 0 for x in self['fishes'].values()]:
print("You caught")
self.dump_cooler
print(print_separator)
if __name__ == '__main__':
"""
This block of code is not executed during import
and instead is usually run when a file is executed,
eg. `python gone_fishing.py`, making it a good
place for simple unit tests and example usage.
"""
gone_fishing = Gone_Fishing(
fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [5]},
'shark': {'amount': 0, 'chances': [9, 10], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [7], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [10, 15], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [2, 19], 'plural': 'tires'},
},
min_chances = 0,
max_chances = 20,
)
gone_fishing.main_loop()
... okay there's a bit going on up there, so feel free to dissect it's operation by adding breakpoints
or print(something)
lines.
Here's what output of running the above script may look like
# python gone_fishing.py
Go fishing? 'yes'
You caught a Wild Fish
Keep fishing? 'yes'
Nothing was caught this time.
Keep fishing? 'yes'
You caught a Shark
You caught a Old Shoe
Keep fishing? 'yes'
Nothing was caught this time.
# ... trimmed for brevity
Keep fishing? 'no'
_________
Thanks for playing
You caught
2 sharks
1 tire
2 wild_fishes
1 cod
_________
Taking it from the top print_separator = "".join(['_' for _ in range(9)])
is what I like to use when generating strings of repeating characters because it's easy to make something that outputs _-_-_
via "-".join(['_' for _ in range(3)])
.
By defining a class that inherits from the built in dict
ionary class
(that's what the class Gone_Fishing(dict):
line did), I'm being a bit lazy as this allows for dumping all saved states via...
print(gone_fishing)
# -> {'cod': {'amount': 2, 'chances': [1]}, ...}
... and while I'm on the tangent of getting info back out...
print(gone_fishing.main_loop.__doc__)
# Or
# help(gone_fishing.main_loop)
... will print the previously mentioned
__doc__
strings.
... and figuring out where you too can avoid re-inventing the wheel is just something that'll get picked up over time. Personally I choose to view it as expanding one's vocabulary, when I discover some built-in that's been waiting to solve some edge-case.
The __init__
method
absorbs three arguments and re-assigns'em with self.update()
so that other methods that use the self
argument are able to get and/or modify class
saved states; more on that latter.
That bit with **kwargs
stands for key word arguments
which passes things as a bare dict
ionary, the other syntax you may run across is *args
, which passes things as a bare list
of arguments; there be some fanciness that can be done with this syntax that I'll not get into at this point. However, you'll find some examples of passing an unwrapped dictionary, such as to format
via print("{amount} {fish}".format(**{...}))
, which hint hint, is a great way of passing variable parameter names.
This is one of those idiomatic things that you can pick-up with some experimentation (and grokking-out others' code bases); it's super powerful so use it often but be kind to your future self too.
The bit with super(Gone_Fishing, self).__init__(**kwargs)
is what allows the Gone_Fishing
class
to call dict
's __init__
from within it's own __init__
method
... indeed that was a little convoluted so taking a sec to unpack that...
class SomeThing(dict):
def __init__(self, an_argument = None, **kwargs):
super(SomeThing, self).__init__(**kwargs)
self.update({'an_argument': an_argument})
... it's possible to call self.update()
from within SomeThing.___init__
without causing confusion of intent, where as to have SomeThing
still operate as a dict
ionary, eg. assigning something = SomeThing(spam = 'Spam')
without causing errors, one should use super(SomeThing, self).__init__(**kwargs)
to allow Python to preform it's voodoo with figuring out which inheriting class
'll take responsibility for those arguments. Side note, that does mean that one could do class SomeThing(dict, Iterator)
, and have that mean something but I'll not get into that here; kinda already covered that specifically on math stack in regards to graph modeling and prioritization.
The @staticmethod
and other decorators
are ways of denoting a special use method
. In the case of property
s they operate similarly to Object
properties, eg...
class Test_Obj:
pass
o = Test_Obj()
o.foo = 'Foo'
print(o.foo)
# -> Foo
... but can only be gotten not set, which makes'em a great place to stash dynamic or semiprivate properties about an object.
In the case of staticmethod
s, they're not passed a reference to self
so cannot easily access or modify saved states, but they can be more easily used without initializing, eg...
responses =
responses.append(Gone_Fishing.question("Where to"))
print("I heard -> {response}".format(response = responses[-1]))
for _ in range(7):
responses.append(Gone_Fishing.question("... are you sure"))
print("I heard -> {response}".format(response = responses[-1]))
print("Okay... though...")
Note also the various
.format()
usages are to show ways of future prepping (for perhaps usingf strings
in the future), as well as making strings somewhat more explicit.
Generally I use'em to make the intended usage more explicit but that's not to say that you couldn't get lost in the amount of options available just for decorating a method
.
That bit within the main_loop
method
with while self.keep_fishing(self.question(message), expected)
, when unwrapped I think you'll really like, it's returning True
or False
at the top of every iteration based on asking the user a question and comparing their response with what's expected.
And the bit with if True in [x['amount'] > 0 for x in self['fishes'].values()]
is something that masks data using list comprehensions
, I'll advise against getting too fancy with'em, and instead try to utilize'em whenever it doesn't make code less readable. Also don't get to attached to such cleverness because numpy
, pandas
, or one of the many other libraries, will preform similar tasks far faster.
The things happening bellow the if __name__ == '__main__':
, aside from the doc string ...
Side note for those new to Python; sure you could call'em "dunder docs" and those in the know would know what you where saying, but they'd also likely smize at ya too, and saying "dundar doc string" if timed when a listener is drinking could have messy consequences... so "pro-tip", callem "doc strings" to be
super
class
y when talking about Python code ;-)
gone_fishing = Gone_Fishing(fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [2]},
'shark': {'amount': 0, 'chances': [3], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [4], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [5, 6], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [7, 8], 'plural': 'tires'},
})
... and how the above is parsed could take some words to do a full stack trace, but the gist is that chances
is a list
that you could even have overlapping integers, eg. a shark
who had an old_shoe
inside could be...
gone_fishing['fishes']['shark']['chances'].append(5)
... though without adjustments to other values that would make for a very large shoal of soul hungry sharks.
Note from the future; I've made adjustments to the code to enable overlapping values and returning of more than one result; there probably be better ways of doing it but this is also an example of iterative development now.
When you've figured out how plural
is an optional key value pair within a nested dictionary you'll start seeing similar things in other code (at least it's one of those things I've not been unable to unsee), try not to get messy with that trick though, otherwise I think it's self-explanatory as to the intentions of it's usage.
The arguments that I didn't assign, min_chance
and max_chance
, much like the chances
with sharks
could be updated similarly, eg...
gone_fishing['chances']['max'] = 20
... though initializing a new trip would look like...
another_fishing_trip = Gone_Fishing(
fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [5]},
'shark': {'amount': 0, 'chances': [9, 10], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [7], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [10, 15], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [2, 19], 'plural': 'tires'},
},
min_chances = 0,
max_chances = 20,
)
... which serves as an example of something you'd be wise to avoid doing to your own code, swapping words especially isn't going to win any points from a future self or other developers.
There's certainly more room for improvement, eg. having gone_fishing['fishes'][fish_name]['ammount']
subtracted from, while adding to gone_fishing['cooler']
or similar structure; just for a start. But this was all just to expose quick-n-dirty methods of organizing the problem space with Object Oriented Programing.
Hopefully having code with a bit more abstraction shows ya that going with something that looks a bit more complex can allow for simplifying the usage and future feature creep
. Please keep us posted if ya make something more out of your learning project.
New contributor
$endgroup$
First off I think your use case is a nifty way of getting into Python, and it looks like aside from the bugs that others have already pointed out you'll likely soon be unstoppable.
However, instead of simplifying the code I'd suggest modularizing as well as making use of __doc__
strings. It'll make adding features much easier in the future, and if you so choose, allow for making a full application with Kivy
, Blender
, or one of the other many GUI frameworks for Python development. Plus modularizing or abstraction allows for simplifying the intentions/usage.
Some notes before diving-in...
it's probably a good idea to get a snack and drink; I'm a bit verbose and am about to compress some years of knowledge
__bar__
when spoken is "dunder bar" , and the phylum that their classified under are "magic methods"
... okay back on track.
Here's some example code inspired by yours that shows some of what I was going on about in your question's comments...
#!/usr/bin/env python
import time
import random
print_separator = "".join(['_' for _ in range(9)])
__author__ = "S0AndS0"
class Gone_Fishing(dict):
"""
Gone_Fishing is a simple simulation inspired by
[Python - Fishing Simulator](https://codereview.stackexchange.com/q/217357/197446)
## Arguments
- `fishes`, `dict`ionary such as `{'cod': {'amount': 0, 'chances': [1, 2]}}`
- `min_chance`, `int`eger of min number that `random.randint` may generate
- `max_chance`, `int`eger of max number that `random.randint` may generate
"""
def __init__(self, fishes, min_chance = 1, max_chance = 10, **kwargs):
super(Gone_Fishing, self).__init__(**kwargs)
self.update(fishes = fishes,
chances = {'min': min_chance, 'max': max_chance})
@staticmethod
def question(message):
""" Returns response to `message` from user """
return input("{message}? ".format(message = message))
@staticmethod
def keep_fishing(response, expected):
""" Return `bool`ean of if `response` matches `expected` """
if not response or not isinstance(response, str):
return False
return response.lower() == expected
@property
def dump_cooler(self):
"""
Returns `score`, a `dict`ionary similar to `{'cod': 5, 'tire': 2}`,
after printing and reseting _`amount`s_ caught
"""
score = {}
for fish, data in self['fishes'].items():
if data['amount'] > 0:
score.update({fish: data['amount']})
if data['amount'] > 1 and data.get('plural'):
fish = data['plural']
print("{amount} {fish}".format(**{
'fish': fish,
'amount': data['amount']}))
data['amount'] = 0
return score
def catch(self, chance):
""" Returns `None` or name of `fish` caught based on `chance` """
caught =
for fish, data in self['fishes'].items():
if chance in data['chances']:
caught.append(fish)
return caught
def main_loop(self):
"""
Asks questions, adds to _cooler_ anything caught, and prints score when finished
"""
first = True
message = 'Go fishing'
expected = 'yes'
while self.keep_fishing(self.question(message), expected):
time.sleep(1)
if first:
first = False
message = "Keep fishing"
chances = random.randint(self['chances']['min'], self['chances']['max'])
caught = self.catch(chances)
if caught:
for fish in caught:
self['fishes'][fish]['amount'] += 1
fancy_fish = ' '.join(fish.split('_')).title()
print("You caught a {fish}".format(fish = fancy_fish))
else:
print("Nothing was caught this time.")
print("{0}nThanks for playing".format(print_separator))
if True in [x['amount'] > 0 for x in self['fishes'].values()]:
print("You caught")
self.dump_cooler
print(print_separator)
if __name__ == '__main__':
"""
This block of code is not executed during import
and instead is usually run when a file is executed,
eg. `python gone_fishing.py`, making it a good
place for simple unit tests and example usage.
"""
gone_fishing = Gone_Fishing(
fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [5]},
'shark': {'amount': 0, 'chances': [9, 10], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [7], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [10, 15], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [2, 19], 'plural': 'tires'},
},
min_chances = 0,
max_chances = 20,
)
gone_fishing.main_loop()
... okay there's a bit going on up there, so feel free to dissect it's operation by adding breakpoints
or print(something)
lines.
Here's what output of running the above script may look like
# python gone_fishing.py
Go fishing? 'yes'
You caught a Wild Fish
Keep fishing? 'yes'
Nothing was caught this time.
Keep fishing? 'yes'
You caught a Shark
You caught a Old Shoe
Keep fishing? 'yes'
Nothing was caught this time.
# ... trimmed for brevity
Keep fishing? 'no'
_________
Thanks for playing
You caught
2 sharks
1 tire
2 wild_fishes
1 cod
_________
Taking it from the top print_separator = "".join(['_' for _ in range(9)])
is what I like to use when generating strings of repeating characters because it's easy to make something that outputs _-_-_
via "-".join(['_' for _ in range(3)])
.
By defining a class that inherits from the built in dict
ionary class
(that's what the class Gone_Fishing(dict):
line did), I'm being a bit lazy as this allows for dumping all saved states via...
print(gone_fishing)
# -> {'cod': {'amount': 2, 'chances': [1]}, ...}
... and while I'm on the tangent of getting info back out...
print(gone_fishing.main_loop.__doc__)
# Or
# help(gone_fishing.main_loop)
... will print the previously mentioned
__doc__
strings.
... and figuring out where you too can avoid re-inventing the wheel is just something that'll get picked up over time. Personally I choose to view it as expanding one's vocabulary, when I discover some built-in that's been waiting to solve some edge-case.
The __init__
method
absorbs three arguments and re-assigns'em with self.update()
so that other methods that use the self
argument are able to get and/or modify class
saved states; more on that latter.
That bit with **kwargs
stands for key word arguments
which passes things as a bare dict
ionary, the other syntax you may run across is *args
, which passes things as a bare list
of arguments; there be some fanciness that can be done with this syntax that I'll not get into at this point. However, you'll find some examples of passing an unwrapped dictionary, such as to format
via print("{amount} {fish}".format(**{...}))
, which hint hint, is a great way of passing variable parameter names.
This is one of those idiomatic things that you can pick-up with some experimentation (and grokking-out others' code bases); it's super powerful so use it often but be kind to your future self too.
The bit with super(Gone_Fishing, self).__init__(**kwargs)
is what allows the Gone_Fishing
class
to call dict
's __init__
from within it's own __init__
method
... indeed that was a little convoluted so taking a sec to unpack that...
class SomeThing(dict):
def __init__(self, an_argument = None, **kwargs):
super(SomeThing, self).__init__(**kwargs)
self.update({'an_argument': an_argument})
... it's possible to call self.update()
from within SomeThing.___init__
without causing confusion of intent, where as to have SomeThing
still operate as a dict
ionary, eg. assigning something = SomeThing(spam = 'Spam')
without causing errors, one should use super(SomeThing, self).__init__(**kwargs)
to allow Python to preform it's voodoo with figuring out which inheriting class
'll take responsibility for those arguments. Side note, that does mean that one could do class SomeThing(dict, Iterator)
, and have that mean something but I'll not get into that here; kinda already covered that specifically on math stack in regards to graph modeling and prioritization.
The @staticmethod
and other decorators
are ways of denoting a special use method
. In the case of property
s they operate similarly to Object
properties, eg...
class Test_Obj:
pass
o = Test_Obj()
o.foo = 'Foo'
print(o.foo)
# -> Foo
... but can only be gotten not set, which makes'em a great place to stash dynamic or semiprivate properties about an object.
In the case of staticmethod
s, they're not passed a reference to self
so cannot easily access or modify saved states, but they can be more easily used without initializing, eg...
responses =
responses.append(Gone_Fishing.question("Where to"))
print("I heard -> {response}".format(response = responses[-1]))
for _ in range(7):
responses.append(Gone_Fishing.question("... are you sure"))
print("I heard -> {response}".format(response = responses[-1]))
print("Okay... though...")
Note also the various
.format()
usages are to show ways of future prepping (for perhaps usingf strings
in the future), as well as making strings somewhat more explicit.
Generally I use'em to make the intended usage more explicit but that's not to say that you couldn't get lost in the amount of options available just for decorating a method
.
That bit within the main_loop
method
with while self.keep_fishing(self.question(message), expected)
, when unwrapped I think you'll really like, it's returning True
or False
at the top of every iteration based on asking the user a question and comparing their response with what's expected.
And the bit with if True in [x['amount'] > 0 for x in self['fishes'].values()]
is something that masks data using list comprehensions
, I'll advise against getting too fancy with'em, and instead try to utilize'em whenever it doesn't make code less readable. Also don't get to attached to such cleverness because numpy
, pandas
, or one of the many other libraries, will preform similar tasks far faster.
The things happening bellow the if __name__ == '__main__':
, aside from the doc string ...
Side note for those new to Python; sure you could call'em "dunder docs" and those in the know would know what you where saying, but they'd also likely smize at ya too, and saying "dundar doc string" if timed when a listener is drinking could have messy consequences... so "pro-tip", callem "doc strings" to be
super
class
y when talking about Python code ;-)
gone_fishing = Gone_Fishing(fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [2]},
'shark': {'amount': 0, 'chances': [3], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [4], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [5, 6], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [7, 8], 'plural': 'tires'},
})
... and how the above is parsed could take some words to do a full stack trace, but the gist is that chances
is a list
that you could even have overlapping integers, eg. a shark
who had an old_shoe
inside could be...
gone_fishing['fishes']['shark']['chances'].append(5)
... though without adjustments to other values that would make for a very large shoal of soul hungry sharks.
Note from the future; I've made adjustments to the code to enable overlapping values and returning of more than one result; there probably be better ways of doing it but this is also an example of iterative development now.
When you've figured out how plural
is an optional key value pair within a nested dictionary you'll start seeing similar things in other code (at least it's one of those things I've not been unable to unsee), try not to get messy with that trick though, otherwise I think it's self-explanatory as to the intentions of it's usage.
The arguments that I didn't assign, min_chance
and max_chance
, much like the chances
with sharks
could be updated similarly, eg...
gone_fishing['chances']['max'] = 20
... though initializing a new trip would look like...
another_fishing_trip = Gone_Fishing(
fishes = {
'cod': {'amount': 0, 'chances': [1]},
'salmon': {'amount': 0, 'chances': [5]},
'shark': {'amount': 0, 'chances': [9, 10], 'plural': 'sharks'},
'wild_fish': {'amount': 0, 'chances': [7], 'plural': 'wild_fishes'},
'old_shoe': {'amount': 0, 'chances': [10, 15], 'plural': 'old_shoes'},
'tire': {'amount': 0, 'chances': [2, 19], 'plural': 'tires'},
},
min_chances = 0,
max_chances = 20,
)
... which serves as an example of something you'd be wise to avoid doing to your own code, swapping words especially isn't going to win any points from a future self or other developers.
There's certainly more room for improvement, eg. having gone_fishing['fishes'][fish_name]['ammount']
subtracted from, while adding to gone_fishing['cooler']
or similar structure; just for a start. But this was all just to expose quick-n-dirty methods of organizing the problem space with Object Oriented Programing.
Hopefully having code with a bit more abstraction shows ya that going with something that looks a bit more complex can allow for simplifying the usage and future feature creep
. Please keep us posted if ya make something more out of your learning project.
New contributor
edited 21 hours ago
New contributor
answered yesterday
S0AndS0S0AndS0
2816
2816
New contributor
New contributor
4
$begingroup$
Welcome to Code Review! I think you did a grat job on your first answer here. Keep it going!
$endgroup$
– Alex
yesterday
1
$begingroup$
A lot of great advice, but I would like to note that this:print_separator = "".join(['_' for _ in range(9)])
Can be shortened to this:print_separator = '_' * 9
$endgroup$
– shellster
yesterday
1
$begingroup$
print("{amount} {fish}".format(**{'fish': fish, 'amount': data['amount']}))
-- why notprint("{amount} {fish}".format(fish=fish, amount=data['amount']))
?
$endgroup$
– kevinsa5
23 hours ago
1
$begingroup$
@S0AndS0 This is obviously a tiny nitpick, but even then it would probably be better to use one of"--".join('_' * 3)
(limitations: underscore must be single character and builds intermediary string),"--".join('_' for _ in range(3))
,"--".join(itertools.repeat('_', 3))
, depending whether or not you likeitertools
. Constructing an intermediary list in memory here is really not necessary.
$endgroup$
– Izaak van Dongen
9 hours ago
1
$begingroup$
Indeed an intermediatelist
is uncalled for in the example use case, and I'm glad that you've pointed out a plethora of other options. That first one is sweet @Izaak van Dongen! And has some interesting outputs when more than one character is used, eg."--".join('_|' * 3)
->
_--|--_--|--_--|
. Especially when compared to the output of your other suggestion"--".join('_|' for _ in range(3))
->
_|--_|--_|
... definitely something that I'll keep in mind for fancy dividers.
$endgroup$
– S0AndS0
8 hours ago
|
show 3 more comments
4
$begingroup$
Welcome to Code Review! I think you did a grat job on your first answer here. Keep it going!
$endgroup$
– Alex
yesterday
1
$begingroup$
A lot of great advice, but I would like to note that this:print_separator = "".join(['_' for _ in range(9)])
Can be shortened to this:print_separator = '_' * 9
$endgroup$
– shellster
yesterday
1
$begingroup$
print("{amount} {fish}".format(**{'fish': fish, 'amount': data['amount']}))
-- why notprint("{amount} {fish}".format(fish=fish, amount=data['amount']))
?
$endgroup$
– kevinsa5
23 hours ago
1
$begingroup$
@S0AndS0 This is obviously a tiny nitpick, but even then it would probably be better to use one of"--".join('_' * 3)
(limitations: underscore must be single character and builds intermediary string),"--".join('_' for _ in range(3))
,"--".join(itertools.repeat('_', 3))
, depending whether or not you likeitertools
. Constructing an intermediary list in memory here is really not necessary.
$endgroup$
– Izaak van Dongen
9 hours ago
1
$begingroup$
Indeed an intermediatelist
is uncalled for in the example use case, and I'm glad that you've pointed out a plethora of other options. That first one is sweet @Izaak van Dongen! And has some interesting outputs when more than one character is used, eg."--".join('_|' * 3)
->
_--|--_--|--_--|
. Especially when compared to the output of your other suggestion"--".join('_|' for _ in range(3))
->
_|--_|--_|
... definitely something that I'll keep in mind for fancy dividers.
$endgroup$
– S0AndS0
8 hours ago
4
4
$begingroup$
Welcome to Code Review! I think you did a grat job on your first answer here. Keep it going!
$endgroup$
– Alex
yesterday
$begingroup$
Welcome to Code Review! I think you did a grat job on your first answer here. Keep it going!
$endgroup$
– Alex
yesterday
1
1
$begingroup$
A lot of great advice, but I would like to note that this:
print_separator = "".join(['_' for _ in range(9)])
Can be shortened to this: print_separator = '_' * 9
$endgroup$
– shellster
yesterday
$begingroup$
A lot of great advice, but I would like to note that this:
print_separator = "".join(['_' for _ in range(9)])
Can be shortened to this: print_separator = '_' * 9
$endgroup$
– shellster
yesterday
1
1
$begingroup$
print("{amount} {fish}".format(**{'fish': fish, 'amount': data['amount']}))
-- why not print("{amount} {fish}".format(fish=fish, amount=data['amount']))
?$endgroup$
– kevinsa5
23 hours ago
$begingroup$
print("{amount} {fish}".format(**{'fish': fish, 'amount': data['amount']}))
-- why not print("{amount} {fish}".format(fish=fish, amount=data['amount']))
?$endgroup$
– kevinsa5
23 hours ago
1
1
$begingroup$
@S0AndS0 This is obviously a tiny nitpick, but even then it would probably be better to use one of
"--".join('_' * 3)
(limitations: underscore must be single character and builds intermediary string), "--".join('_' for _ in range(3))
, "--".join(itertools.repeat('_', 3))
, depending whether or not you like itertools
. Constructing an intermediary list in memory here is really not necessary.$endgroup$
– Izaak van Dongen
9 hours ago
$begingroup$
@S0AndS0 This is obviously a tiny nitpick, but even then it would probably be better to use one of
"--".join('_' * 3)
(limitations: underscore must be single character and builds intermediary string), "--".join('_' for _ in range(3))
, "--".join(itertools.repeat('_', 3))
, depending whether or not you like itertools
. Constructing an intermediary list in memory here is really not necessary.$endgroup$
– Izaak van Dongen
9 hours ago
1
1
$begingroup$
Indeed an intermediate
list
is uncalled for in the example use case, and I'm glad that you've pointed out a plethora of other options. That first one is sweet @Izaak van Dongen! And has some interesting outputs when more than one character is used, eg. "--".join('_|' * 3)
->
_--|--_--|--_--|
. Especially when compared to the output of your other suggestion "--".join('_|' for _ in range(3))
->
_|--_|--_|
... definitely something that I'll keep in mind for fancy dividers.$endgroup$
– S0AndS0
8 hours ago
$begingroup$
Indeed an intermediate
list
is uncalled for in the example use case, and I'm glad that you've pointed out a plethora of other options. That first one is sweet @Izaak van Dongen! And has some interesting outputs when more than one character is used, eg. "--".join('_|' * 3)
->
_--|--_--|--_--|
. Especially when compared to the output of your other suggestion "--".join('_|' for _ in range(3))
->
_|--_|--_|
... definitely something that I'll keep in mind for fancy dividers.$endgroup$
– S0AndS0
8 hours ago
|
show 3 more comments
$begingroup$
This is another inprovement using a dictionary. Currently all of your data is hardcoded and distributed somewhere in the code. If you wanted to add another fish, you would have to add a variable f
, extend random.randint
(so the chance for nothing does not decrease) and finally add it to the if
conditions and the printing.
That is a lot of work just to add one more fish. Instead I would propose to use a dictionary of possible fishing outcomes and their chance of being caught. You can then use this with random.choices
, which takes a weights
argument detailing the probabilities.
pond = {'cod': 1, 'salmon': 1, 'shark': 1, 'wildfish': 1, 'nothing': 2}
The probabilities are here just relative to each other, random.choices
normalizes them for you. All fish have the same probability and getting nothing has double the probability of any single fish.
Your loop also does not need the fishing
variable at all, just break
it when the user is done fishing.
Whenever you need to count something, using collections.Counter
is probably a good idea. It basically works like a dictionary and has the nice feature that it assumes all elements have a count of zero.
In Python 3.6 a new way to format strings was introduced, the f-string
.
from collections import Counter
from random import choices
from time import sleep
POND = {'cod': 1, 'salmon': 1, 'shark': 1, 'wildfish': 1, 'nothing': 2}
name = input("What is your name fisherman? ")
caught = Counter()
while True:
keep_fishing = input("Throw out your line, or go home? ")
if keep_fishing == "go home":
break
sleep(1)
result = choices(list(POND), weights=POND.values(), k=1)[0]
print(f"You caught: {result}")
caught[result] += 1
print(f"nThanks for playing, {name}!")
print("You caught:")
for fish, n in caught.most_common():
if fish != "nothing":
print(n, fish)
$endgroup$
$begingroup$
For the behavior to be the same as the original code, nothing should have chance 3
$endgroup$
– Vaelus
6 hours ago
$begingroup$
@Vaelusrandrange
is exclusive of the end, so produces vales from 1 to 6 (inclusive), therefore only 5 and 6 producenothing
.
$endgroup$
– Graipher
5 hours ago
$begingroup$
Whoops, you're right. Yet another reason to use your method.
$endgroup$
– Vaelus
3 hours ago
add a comment |
$begingroup$
This is another inprovement using a dictionary. Currently all of your data is hardcoded and distributed somewhere in the code. If you wanted to add another fish, you would have to add a variable f
, extend random.randint
(so the chance for nothing does not decrease) and finally add it to the if
conditions and the printing.
That is a lot of work just to add one more fish. Instead I would propose to use a dictionary of possible fishing outcomes and their chance of being caught. You can then use this with random.choices
, which takes a weights
argument detailing the probabilities.
pond = {'cod': 1, 'salmon': 1, 'shark': 1, 'wildfish': 1, 'nothing': 2}
The probabilities are here just relative to each other, random.choices
normalizes them for you. All fish have the same probability and getting nothing has double the probability of any single fish.
Your loop also does not need the fishing
variable at all, just break
it when the user is done fishing.
Whenever you need to count something, using collections.Counter
is probably a good idea. It basically works like a dictionary and has the nice feature that it assumes all elements have a count of zero.
In Python 3.6 a new way to format strings was introduced, the f-string
.
from collections import Counter
from random import choices
from time import sleep
POND = {'cod': 1, 'salmon': 1, 'shark': 1, 'wildfish': 1, 'nothing': 2}
name = input("What is your name fisherman? ")
caught = Counter()
while True:
keep_fishing = input("Throw out your line, or go home? ")
if keep_fishing == "go home":
break
sleep(1)
result = choices(list(POND), weights=POND.values(), k=1)[0]
print(f"You caught: {result}")
caught[result] += 1
print(f"nThanks for playing, {name}!")
print("You caught:")
for fish, n in caught.most_common():
if fish != "nothing":
print(n, fish)
$endgroup$
$begingroup$
For the behavior to be the same as the original code, nothing should have chance 3
$endgroup$
– Vaelus
6 hours ago
$begingroup$
@Vaelusrandrange
is exclusive of the end, so produces vales from 1 to 6 (inclusive), therefore only 5 and 6 producenothing
.
$endgroup$
– Graipher
5 hours ago
$begingroup$
Whoops, you're right. Yet another reason to use your method.
$endgroup$
– Vaelus
3 hours ago
add a comment |
$begingroup$
This is another inprovement using a dictionary. Currently all of your data is hardcoded and distributed somewhere in the code. If you wanted to add another fish, you would have to add a variable f
, extend random.randint
(so the chance for nothing does not decrease) and finally add it to the if
conditions and the printing.
That is a lot of work just to add one more fish. Instead I would propose to use a dictionary of possible fishing outcomes and their chance of being caught. You can then use this with random.choices
, which takes a weights
argument detailing the probabilities.
pond = {'cod': 1, 'salmon': 1, 'shark': 1, 'wildfish': 1, 'nothing': 2}
The probabilities are here just relative to each other, random.choices
normalizes them for you. All fish have the same probability and getting nothing has double the probability of any single fish.
Your loop also does not need the fishing
variable at all, just break
it when the user is done fishing.
Whenever you need to count something, using collections.Counter
is probably a good idea. It basically works like a dictionary and has the nice feature that it assumes all elements have a count of zero.
In Python 3.6 a new way to format strings was introduced, the f-string
.
from collections import Counter
from random import choices
from time import sleep
POND = {'cod': 1, 'salmon': 1, 'shark': 1, 'wildfish': 1, 'nothing': 2}
name = input("What is your name fisherman? ")
caught = Counter()
while True:
keep_fishing = input("Throw out your line, or go home? ")
if keep_fishing == "go home":
break
sleep(1)
result = choices(list(POND), weights=POND.values(), k=1)[0]
print(f"You caught: {result}")
caught[result] += 1
print(f"nThanks for playing, {name}!")
print("You caught:")
for fish, n in caught.most_common():
if fish != "nothing":
print(n, fish)
$endgroup$
This is another inprovement using a dictionary. Currently all of your data is hardcoded and distributed somewhere in the code. If you wanted to add another fish, you would have to add a variable f
, extend random.randint
(so the chance for nothing does not decrease) and finally add it to the if
conditions and the printing.
That is a lot of work just to add one more fish. Instead I would propose to use a dictionary of possible fishing outcomes and their chance of being caught. You can then use this with random.choices
, which takes a weights
argument detailing the probabilities.
pond = {'cod': 1, 'salmon': 1, 'shark': 1, 'wildfish': 1, 'nothing': 2}
The probabilities are here just relative to each other, random.choices
normalizes them for you. All fish have the same probability and getting nothing has double the probability of any single fish.
Your loop also does not need the fishing
variable at all, just break
it when the user is done fishing.
Whenever you need to count something, using collections.Counter
is probably a good idea. It basically works like a dictionary and has the nice feature that it assumes all elements have a count of zero.
In Python 3.6 a new way to format strings was introduced, the f-string
.
from collections import Counter
from random import choices
from time import sleep
POND = {'cod': 1, 'salmon': 1, 'shark': 1, 'wildfish': 1, 'nothing': 2}
name = input("What is your name fisherman? ")
caught = Counter()
while True:
keep_fishing = input("Throw out your line, or go home? ")
if keep_fishing == "go home":
break
sleep(1)
result = choices(list(POND), weights=POND.values(), k=1)[0]
print(f"You caught: {result}")
caught[result] += 1
print(f"nThanks for playing, {name}!")
print("You caught:")
for fish, n in caught.most_common():
if fish != "nothing":
print(n, fish)
edited 9 hours ago
answered yesterday
GraipherGraipher
27.2k54497
27.2k54497
$begingroup$
For the behavior to be the same as the original code, nothing should have chance 3
$endgroup$
– Vaelus
6 hours ago
$begingroup$
@Vaelusrandrange
is exclusive of the end, so produces vales from 1 to 6 (inclusive), therefore only 5 and 6 producenothing
.
$endgroup$
– Graipher
5 hours ago
$begingroup$
Whoops, you're right. Yet another reason to use your method.
$endgroup$
– Vaelus
3 hours ago
add a comment |
$begingroup$
For the behavior to be the same as the original code, nothing should have chance 3
$endgroup$
– Vaelus
6 hours ago
$begingroup$
@Vaelusrandrange
is exclusive of the end, so produces vales from 1 to 6 (inclusive), therefore only 5 and 6 producenothing
.
$endgroup$
– Graipher
5 hours ago
$begingroup$
Whoops, you're right. Yet another reason to use your method.
$endgroup$
– Vaelus
3 hours ago
$begingroup$
For the behavior to be the same as the original code, nothing should have chance 3
$endgroup$
– Vaelus
6 hours ago
$begingroup$
For the behavior to be the same as the original code, nothing should have chance 3
$endgroup$
– Vaelus
6 hours ago
$begingroup$
@Vaelus
randrange
is exclusive of the end, so produces vales from 1 to 6 (inclusive), therefore only 5 and 6 produce nothing
.$endgroup$
– Graipher
5 hours ago
$begingroup$
@Vaelus
randrange
is exclusive of the end, so produces vales from 1 to 6 (inclusive), therefore only 5 and 6 produce nothing
.$endgroup$
– Graipher
5 hours ago
$begingroup$
Whoops, you're right. Yet another reason to use your method.
$endgroup$
– Vaelus
3 hours ago
$begingroup$
Whoops, you're right. Yet another reason to use your method.
$endgroup$
– Vaelus
3 hours ago
add a comment |
$begingroup$
In addition to the other answers, you can also take advantage of python dictionaries:
a = b = c = d = e = 0
...
else:
t = random.randrange(1, 7)
if t == 1:
a += 1
print("You caught a cod!")
elif t == 2:
b += 1
print("You caught a salmon!")
elif t == 3:
c += 1
print("You caught a shark!")
elif t == 4:
d += 1
print("You caught a wildfish!")
elif t >= 5:
e += 1
print("You caught nothing!")
Becomes:
caught_fish = {
'cod': 0,
'salmon': 0,
'shark': 0,
'wildfish': 0,
'nothing': 0,
}
...
else:
t = random.randrange(1,7)
# clamp 't' to dictionary size
if t > len(caught_fish):
t = len(caught_fish)
# pick a type of fish from the list of keys of 'caught_fish' using index 't'
type_of_fish = list(caught_fish)[t - 1]
# update the dictionary
caught_fish[type_of_fish] += 1
# print what type of fish was caught, or if no fish was caught
article = 'a ' if type_of_fish != 'nothing' else ''
print("You caught {}{}!".format(article, type_of_fish))
$endgroup$
add a comment |
$begingroup$
In addition to the other answers, you can also take advantage of python dictionaries:
a = b = c = d = e = 0
...
else:
t = random.randrange(1, 7)
if t == 1:
a += 1
print("You caught a cod!")
elif t == 2:
b += 1
print("You caught a salmon!")
elif t == 3:
c += 1
print("You caught a shark!")
elif t == 4:
d += 1
print("You caught a wildfish!")
elif t >= 5:
e += 1
print("You caught nothing!")
Becomes:
caught_fish = {
'cod': 0,
'salmon': 0,
'shark': 0,
'wildfish': 0,
'nothing': 0,
}
...
else:
t = random.randrange(1,7)
# clamp 't' to dictionary size
if t > len(caught_fish):
t = len(caught_fish)
# pick a type of fish from the list of keys of 'caught_fish' using index 't'
type_of_fish = list(caught_fish)[t - 1]
# update the dictionary
caught_fish[type_of_fish] += 1
# print what type of fish was caught, or if no fish was caught
article = 'a ' if type_of_fish != 'nothing' else ''
print("You caught {}{}!".format(article, type_of_fish))
$endgroup$
add a comment |
$begingroup$
In addition to the other answers, you can also take advantage of python dictionaries:
a = b = c = d = e = 0
...
else:
t = random.randrange(1, 7)
if t == 1:
a += 1
print("You caught a cod!")
elif t == 2:
b += 1
print("You caught a salmon!")
elif t == 3:
c += 1
print("You caught a shark!")
elif t == 4:
d += 1
print("You caught a wildfish!")
elif t >= 5:
e += 1
print("You caught nothing!")
Becomes:
caught_fish = {
'cod': 0,
'salmon': 0,
'shark': 0,
'wildfish': 0,
'nothing': 0,
}
...
else:
t = random.randrange(1,7)
# clamp 't' to dictionary size
if t > len(caught_fish):
t = len(caught_fish)
# pick a type of fish from the list of keys of 'caught_fish' using index 't'
type_of_fish = list(caught_fish)[t - 1]
# update the dictionary
caught_fish[type_of_fish] += 1
# print what type of fish was caught, or if no fish was caught
article = 'a ' if type_of_fish != 'nothing' else ''
print("You caught {}{}!".format(article, type_of_fish))
$endgroup$
In addition to the other answers, you can also take advantage of python dictionaries:
a = b = c = d = e = 0
...
else:
t = random.randrange(1, 7)
if t == 1:
a += 1
print("You caught a cod!")
elif t == 2:
b += 1
print("You caught a salmon!")
elif t == 3:
c += 1
print("You caught a shark!")
elif t == 4:
d += 1
print("You caught a wildfish!")
elif t >= 5:
e += 1
print("You caught nothing!")
Becomes:
caught_fish = {
'cod': 0,
'salmon': 0,
'shark': 0,
'wildfish': 0,
'nothing': 0,
}
...
else:
t = random.randrange(1,7)
# clamp 't' to dictionary size
if t > len(caught_fish):
t = len(caught_fish)
# pick a type of fish from the list of keys of 'caught_fish' using index 't'
type_of_fish = list(caught_fish)[t - 1]
# update the dictionary
caught_fish[type_of_fish] += 1
# print what type of fish was caught, or if no fish was caught
article = 'a ' if type_of_fish != 'nothing' else ''
print("You caught {}{}!".format(article, type_of_fish))
edited yesterday
answered yesterday
VaelusVaelus
1614
1614
add a comment |
add a comment |
Mattthecommie is a new contributor. Be nice, and check out our Code of Conduct.
Mattthecommie is a new contributor. Be nice, and check out our Code of Conduct.
Mattthecommie is a new contributor. Be nice, and check out our Code of Conduct.
Mattthecommie is a new contributor. Be nice, and check out our Code of Conduct.
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f217357%2ffishing-simulator%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
3
$begingroup$
Why did you tag this question as python-2.x? I'm not convinced that it would work correctly in Python 2.
$endgroup$
– 200_success
2 days ago