Fishing simulator Planned maintenance scheduled April 23, 2019 at 00:00UTC (8:00pm US/Eastern) Announcing the arrival of Valued Associate #679: Cesar Manara Unicorn Meta Zoo #1: Why another podcast?Turn-based battle simulatorBattle simulator RPGScoring and grading answers against an answer keyMonopoly simulatorSimple meter simulatorC++ Quiz game w/questions in random orderRandom MP3 Selector and Player“The Story of a Tree” solved using depth-first searchCoin flip simulator with GUIGame of Life simulator, Python 3

How fail-safe is nr as stop bytes?

How much damage would a cupful of neutron star matter do to the Earth?

Amount of permutations on an NxNxN Rubik's Cube

Why is it faster to reheat something than it is to cook it?

How to play a character with a disability or mental disorder without being offensive?

What was the first language to use conditional keywords?

What is a fractional matching?

How do I find out the mythology and history of my Fortress?

Question about debouncing - delay of state change

Is grep documentation about ignoring case wrong, since it doesn't ignore case in filenames?

Maximum summed subsequences with non-adjacent items

What is this clumpy 20-30cm high yellow-flowered plant?

How to tell that you are a giant?

Do I really need to have a message in a novel to appeal to readers?

Why do we need to use the builder design pattern when we can do the same thing with setters?

Generate an RGB colour grid

How would a mousetrap for use in space work?

AppleTVs create a chatty alternate WiFi network

How do I use the new nonlinear finite element in Mathematica 12 for this equation?

How do living politicians protect their readily obtainable signatures from misuse?

Morning, Afternoon, Night Kanji

What is the appropriate index architecture when forced to implement IsDeleted (soft deletes)?

Performance gap between vector<bool> and array

Trademark violation for app?



Fishing simulator



Planned maintenance scheduled April 23, 2019 at 00:00UTC (8:00pm US/Eastern)
Announcing the arrival of Valued Associate #679: Cesar Manara
Unicorn Meta Zoo #1: Why another podcast?Turn-based battle simulatorBattle simulator RPGScoring and grading answers against an answer keyMonopoly simulatorSimple meter simulatorC++ Quiz game w/questions in random orderRandom MP3 Selector and Player“The Story of a Tree” solved using depth-first searchCoin flip simulator with GUIGame of Life simulator, Python 3



.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;








25












$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!")









share|improve this question









New contributor




Mattthecommie is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.







$endgroup$







  • 6




    $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
    Apr 13 at 0:01

















25












$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!")









share|improve this question









New contributor




Mattthecommie is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.







$endgroup$







  • 6




    $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
    Apr 13 at 0:01













25












25








25


5



$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!")









share|improve this question









New contributor




Mattthecommie is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.







$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






share|improve this question









New contributor




Mattthecommie is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.











share|improve this question









New contributor




Mattthecommie is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.









share|improve this question




share|improve this question








edited Apr 14 at 15:52









Stephen Rauch

3,77061630




3,77061630






New contributor




Mattthecommie is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.









asked Apr 12 at 23:30









MattthecommieMattthecommie

18127




18127




New contributor




Mattthecommie is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.





New contributor





Mattthecommie is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.






Mattthecommie is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.







  • 6




    $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
    Apr 13 at 0:01












  • 6




    $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
    Apr 13 at 0:01







6




6




$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
Apr 13 at 0:01




$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
Apr 13 at 0:01










5 Answers
5






active

oldest

votes


















32












$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, ...).






share|improve this answer









$endgroup$




















    24












    $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 exactly c = ... 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.






    share|improve this answer











    $endgroup$












    • $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
      Apr 14 at 6:32










    • $begingroup$
      if answer.lower()[0] == "n": will break if answer is an empty string. Better if answer.lower().startswith('n'):. :-)
      $endgroup$
      – TrebledJ
      Apr 14 at 7:20










    • $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
      Apr 14 at 13:13










    • $begingroup$
      @TrebledJ Yes, good call.
      $endgroup$
      – Carcigenicate
      Apr 14 at 13:13










    • $begingroup$
      @niko no, don't do this. Defensive coding is a good paradigm, but it has to do with handling unknown inputs, not elements of your own code. while fishing is the correct statement here.
      $endgroup$
      – Adam Smith
      Apr 15 at 15:57


















    22












    $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 they're classified under are "magic methods"


    • what I share is not gospel as such, but a collection of tricks I wish someone had shown me when I was getting into Python


    ... 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"

    #
    # Functions
    #

    def question(message):
    """ Returns response to `message` from user """
    return input("message? ".format(message = message))


    #
    # Classes
    #

    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 keep_fishing(message, expected):
    """ Return `bool`ean of if `response` to `message` matches `expected` """
    response = question(message)
    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(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("0nThanks 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)]).




    Note from the future; check the comments of this answer for some swell suggestions from @Izaak van Dongen.





    By defining a class that inherits from the built in dictionary 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.




    Side note; the __init__ method is one of many that are called implicitly by preforming some action with an object, eg. __add__ is called implicitly by using + between two Objects with a __add__ method (side-side note, I'll get into why that was an a and not an an in a bit), which is why the following works with lists...




    list_one = [3, 2, 1]
    list_two = [0, -1, -2]

    list_one + list_two
    # -> [3, 2, 1, 0, -1, -2]


    That bit with **kwargs stands for key word arguments which passes things as a bare dictionary, 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 other than saying that context matters. 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 dictionary, 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.




    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 propertys 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 staticmethods, 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 so operate similarly to regular functions, eg...



    responses = []

    responses.append(question("Where to"))
    print("I heard -> response".format(response = responses[-1]))
    for _ in range(7):
    responses.append(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 using f 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.




    Note from the future; as pointed out by @Maarten Fabré I indeed slipped in some superfluous use of the staticmethod decorator, good catch there, and this'll now serve as an example of getting carried away when decorating.



    Generally I use staticmethods when I've a class that isn't concerned with it's internal state but isn't large enough to warrant it's own file, very edge case kinda thing, and usually it means that I should probably split'em out into a file that organizes similar functions. Hopefully recent edits now look closer to proper for future readers.





    That bit within the main_loop method with while self.keep_fishing(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 classy 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]['amount'] 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.






    share|improve this answer










    New contributor




    S0AndS0 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
    Check out our Code of Conduct.






    $endgroup$








    • 5




      $begingroup$
      Welcome to Code Review! I think you did a grat job on your first answer here. Keep it going!
      $endgroup$
      – Alex
      Apr 13 at 8:01






    • 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
      Apr 13 at 23:06







    • 1




      $begingroup$
      print("amount fish".format(**'fish': fish, 'amount': data['amount'])) -- why not print("amount fish".format(fish=fish, amount=data['amount']))?
      $endgroup$
      – kevinsa5
      Apr 14 at 3:20






    • 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
      Apr 14 at 17:41






    • 2




      $begingroup$
      If you really want to use classes (do you?), I would expect a class for a Fish, a Pond, a Fisher and perhaps a Game. Not like this. All these staticmethods are unneeded on the class, but should go in the module namespace. You make a lot of simple things extra complicated. I suggest you post this piece of code as a seperate question, then you can get some tips on how to improve this.
      $endgroup$
      – Maarten Fabré
      Apr 15 at 8:59


















    10












    $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)





    share|improve this answer











    $endgroup$












    • $begingroup$
      For the behavior to be the same as the original code, nothing should have chance 3
      $endgroup$
      – Vaelus
      Apr 14 at 20:42










    • $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
      Apr 14 at 20:45






    • 1




      $begingroup$
      Whoops, you're right. Yet another reason to use your method.
      $endgroup$
      – Vaelus
      Apr 14 at 23:36


















    6












    $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))





    share|improve this answer











    $endgroup$













      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.









      draft saved

      draft discarded


















      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









      32












      $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, ...).






      share|improve this answer









      $endgroup$

















        32












        $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, ...).






        share|improve this answer









        $endgroup$















          32












          32








          32





          $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, ...).






          share|improve this answer









          $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, ...).







          share|improve this answer












          share|improve this answer



          share|improve this answer










          answered Apr 13 at 1:01









          Austin HastingsAustin Hastings

          8,1971337




          8,1971337























              24












              $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 exactly c = ... 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.






              share|improve this answer











              $endgroup$












              • $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
                Apr 14 at 6:32










              • $begingroup$
                if answer.lower()[0] == "n": will break if answer is an empty string. Better if answer.lower().startswith('n'):. :-)
                $endgroup$
                – TrebledJ
                Apr 14 at 7:20










              • $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
                Apr 14 at 13:13










              • $begingroup$
                @TrebledJ Yes, good call.
                $endgroup$
                – Carcigenicate
                Apr 14 at 13:13










              • $begingroup$
                @niko no, don't do this. Defensive coding is a good paradigm, but it has to do with handling unknown inputs, not elements of your own code. while fishing is the correct statement here.
                $endgroup$
                – Adam Smith
                Apr 15 at 15:57















              24












              $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 exactly c = ... 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.






              share|improve this answer











              $endgroup$












              • $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
                Apr 14 at 6:32










              • $begingroup$
                if answer.lower()[0] == "n": will break if answer is an empty string. Better if answer.lower().startswith('n'):. :-)
                $endgroup$
                – TrebledJ
                Apr 14 at 7:20










              • $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
                Apr 14 at 13:13










              • $begingroup$
                @TrebledJ Yes, good call.
                $endgroup$
                – Carcigenicate
                Apr 14 at 13:13










              • $begingroup$
                @niko no, don't do this. Defensive coding is a good paradigm, but it has to do with handling unknown inputs, not elements of your own code. while fishing is the correct statement here.
                $endgroup$
                – Adam Smith
                Apr 15 at 15:57













              24












              24








              24





              $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 exactly c = ... 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.






              share|improve this answer











              $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 exactly c = ... 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.







              share|improve this answer














              share|improve this answer



              share|improve this answer








              edited Apr 14 at 13:15

























              answered Apr 13 at 0:47









              CarcigenicateCarcigenicate

              4,40411635




              4,40411635











              • $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
                Apr 14 at 6:32










              • $begingroup$
                if answer.lower()[0] == "n": will break if answer is an empty string. Better if answer.lower().startswith('n'):. :-)
                $endgroup$
                – TrebledJ
                Apr 14 at 7:20










              • $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
                Apr 14 at 13:13










              • $begingroup$
                @TrebledJ Yes, good call.
                $endgroup$
                – Carcigenicate
                Apr 14 at 13:13










              • $begingroup$
                @niko no, don't do this. Defensive coding is a good paradigm, but it has to do with handling unknown inputs, not elements of your own code. while fishing is the correct statement here.
                $endgroup$
                – Adam Smith
                Apr 15 at 15:57
















              • $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
                Apr 14 at 6:32










              • $begingroup$
                if answer.lower()[0] == "n": will break if answer is an empty string. Better if answer.lower().startswith('n'):. :-)
                $endgroup$
                – TrebledJ
                Apr 14 at 7:20










              • $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
                Apr 14 at 13:13










              • $begingroup$
                @TrebledJ Yes, good call.
                $endgroup$
                – Carcigenicate
                Apr 14 at 13:13










              • $begingroup$
                @niko no, don't do this. Defensive coding is a good paradigm, but it has to do with handling unknown inputs, not elements of your own code. while fishing is the correct statement here.
                $endgroup$
                – Adam Smith
                Apr 15 at 15:57















              $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
              Apr 14 at 6:32




              $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
              Apr 14 at 6:32












              $begingroup$
              if answer.lower()[0] == "n": will break if answer is an empty string. Better if answer.lower().startswith('n'):. :-)
              $endgroup$
              – TrebledJ
              Apr 14 at 7:20




              $begingroup$
              if answer.lower()[0] == "n": will break if answer is an empty string. Better if answer.lower().startswith('n'):. :-)
              $endgroup$
              – TrebledJ
              Apr 14 at 7:20












              $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
              Apr 14 at 13:13




              $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
              Apr 14 at 13:13












              $begingroup$
              @TrebledJ Yes, good call.
              $endgroup$
              – Carcigenicate
              Apr 14 at 13:13




              $begingroup$
              @TrebledJ Yes, good call.
              $endgroup$
              – Carcigenicate
              Apr 14 at 13:13












              $begingroup$
              @niko no, don't do this. Defensive coding is a good paradigm, but it has to do with handling unknown inputs, not elements of your own code. while fishing is the correct statement here.
              $endgroup$
              – Adam Smith
              Apr 15 at 15:57




              $begingroup$
              @niko no, don't do this. Defensive coding is a good paradigm, but it has to do with handling unknown inputs, not elements of your own code. while fishing is the correct statement here.
              $endgroup$
              – Adam Smith
              Apr 15 at 15:57











              22












              $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 they're classified under are "magic methods"


              • what I share is not gospel as such, but a collection of tricks I wish someone had shown me when I was getting into Python


              ... 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"

              #
              # Functions
              #

              def question(message):
              """ Returns response to `message` from user """
              return input("message? ".format(message = message))


              #
              # Classes
              #

              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 keep_fishing(message, expected):
              """ Return `bool`ean of if `response` to `message` matches `expected` """
              response = question(message)
              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(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("0nThanks 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)]).




              Note from the future; check the comments of this answer for some swell suggestions from @Izaak van Dongen.





              By defining a class that inherits from the built in dictionary 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.




              Side note; the __init__ method is one of many that are called implicitly by preforming some action with an object, eg. __add__ is called implicitly by using + between two Objects with a __add__ method (side-side note, I'll get into why that was an a and not an an in a bit), which is why the following works with lists...




              list_one = [3, 2, 1]
              list_two = [0, -1, -2]

              list_one + list_two
              # -> [3, 2, 1, 0, -1, -2]


              That bit with **kwargs stands for key word arguments which passes things as a bare dictionary, 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 other than saying that context matters. 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 dictionary, 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.




              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 propertys 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 staticmethods, 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 so operate similarly to regular functions, eg...



              responses = []

              responses.append(question("Where to"))
              print("I heard -> response".format(response = responses[-1]))
              for _ in range(7):
              responses.append(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 using f 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.




              Note from the future; as pointed out by @Maarten Fabré I indeed slipped in some superfluous use of the staticmethod decorator, good catch there, and this'll now serve as an example of getting carried away when decorating.



              Generally I use staticmethods when I've a class that isn't concerned with it's internal state but isn't large enough to warrant it's own file, very edge case kinda thing, and usually it means that I should probably split'em out into a file that organizes similar functions. Hopefully recent edits now look closer to proper for future readers.





              That bit within the main_loop method with while self.keep_fishing(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 classy 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]['amount'] 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.






              share|improve this answer










              New contributor




              S0AndS0 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
              Check out our Code of Conduct.






              $endgroup$








              • 5




                $begingroup$
                Welcome to Code Review! I think you did a grat job on your first answer here. Keep it going!
                $endgroup$
                – Alex
                Apr 13 at 8:01






              • 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
                Apr 13 at 23:06







              • 1




                $begingroup$
                print("amount fish".format(**'fish': fish, 'amount': data['amount'])) -- why not print("amount fish".format(fish=fish, amount=data['amount']))?
                $endgroup$
                – kevinsa5
                Apr 14 at 3:20






              • 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
                Apr 14 at 17:41






              • 2




                $begingroup$
                If you really want to use classes (do you?), I would expect a class for a Fish, a Pond, a Fisher and perhaps a Game. Not like this. All these staticmethods are unneeded on the class, but should go in the module namespace. You make a lot of simple things extra complicated. I suggest you post this piece of code as a seperate question, then you can get some tips on how to improve this.
                $endgroup$
                – Maarten Fabré
                Apr 15 at 8:59















              22












              $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 they're classified under are "magic methods"


              • what I share is not gospel as such, but a collection of tricks I wish someone had shown me when I was getting into Python


              ... 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"

              #
              # Functions
              #

              def question(message):
              """ Returns response to `message` from user """
              return input("message? ".format(message = message))


              #
              # Classes
              #

              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 keep_fishing(message, expected):
              """ Return `bool`ean of if `response` to `message` matches `expected` """
              response = question(message)
              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(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("0nThanks 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)]).




              Note from the future; check the comments of this answer for some swell suggestions from @Izaak van Dongen.





              By defining a class that inherits from the built in dictionary 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.




              Side note; the __init__ method is one of many that are called implicitly by preforming some action with an object, eg. __add__ is called implicitly by using + between two Objects with a __add__ method (side-side note, I'll get into why that was an a and not an an in a bit), which is why the following works with lists...




              list_one = [3, 2, 1]
              list_two = [0, -1, -2]

              list_one + list_two
              # -> [3, 2, 1, 0, -1, -2]


              That bit with **kwargs stands for key word arguments which passes things as a bare dictionary, 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 other than saying that context matters. 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 dictionary, 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.




              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 propertys 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 staticmethods, 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 so operate similarly to regular functions, eg...



              responses = []

              responses.append(question("Where to"))
              print("I heard -> response".format(response = responses[-1]))
              for _ in range(7):
              responses.append(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 using f 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.




              Note from the future; as pointed out by @Maarten Fabré I indeed slipped in some superfluous use of the staticmethod decorator, good catch there, and this'll now serve as an example of getting carried away when decorating.



              Generally I use staticmethods when I've a class that isn't concerned with it's internal state but isn't large enough to warrant it's own file, very edge case kinda thing, and usually it means that I should probably split'em out into a file that organizes similar functions. Hopefully recent edits now look closer to proper for future readers.





              That bit within the main_loop method with while self.keep_fishing(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 classy 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]['amount'] 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.






              share|improve this answer










              New contributor




              S0AndS0 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
              Check out our Code of Conduct.






              $endgroup$








              • 5




                $begingroup$
                Welcome to Code Review! I think you did a grat job on your first answer here. Keep it going!
                $endgroup$
                – Alex
                Apr 13 at 8:01






              • 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
                Apr 13 at 23:06







              • 1




                $begingroup$
                print("amount fish".format(**'fish': fish, 'amount': data['amount'])) -- why not print("amount fish".format(fish=fish, amount=data['amount']))?
                $endgroup$
                – kevinsa5
                Apr 14 at 3:20






              • 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
                Apr 14 at 17:41






              • 2




                $begingroup$
                If you really want to use classes (do you?), I would expect a class for a Fish, a Pond, a Fisher and perhaps a Game. Not like this. All these staticmethods are unneeded on the class, but should go in the module namespace. You make a lot of simple things extra complicated. I suggest you post this piece of code as a seperate question, then you can get some tips on how to improve this.
                $endgroup$
                – Maarten Fabré
                Apr 15 at 8:59













              22












              22








              22





              $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 they're classified under are "magic methods"


              • what I share is not gospel as such, but a collection of tricks I wish someone had shown me when I was getting into Python


              ... 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"

              #
              # Functions
              #

              def question(message):
              """ Returns response to `message` from user """
              return input("message? ".format(message = message))


              #
              # Classes
              #

              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 keep_fishing(message, expected):
              """ Return `bool`ean of if `response` to `message` matches `expected` """
              response = question(message)
              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(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("0nThanks 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)]).




              Note from the future; check the comments of this answer for some swell suggestions from @Izaak van Dongen.





              By defining a class that inherits from the built in dictionary 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.




              Side note; the __init__ method is one of many that are called implicitly by preforming some action with an object, eg. __add__ is called implicitly by using + between two Objects with a __add__ method (side-side note, I'll get into why that was an a and not an an in a bit), which is why the following works with lists...




              list_one = [3, 2, 1]
              list_two = [0, -1, -2]

              list_one + list_two
              # -> [3, 2, 1, 0, -1, -2]


              That bit with **kwargs stands for key word arguments which passes things as a bare dictionary, 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 other than saying that context matters. 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 dictionary, 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.




              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 propertys 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 staticmethods, 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 so operate similarly to regular functions, eg...



              responses = []

              responses.append(question("Where to"))
              print("I heard -> response".format(response = responses[-1]))
              for _ in range(7):
              responses.append(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 using f 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.




              Note from the future; as pointed out by @Maarten Fabré I indeed slipped in some superfluous use of the staticmethod decorator, good catch there, and this'll now serve as an example of getting carried away when decorating.



              Generally I use staticmethods when I've a class that isn't concerned with it's internal state but isn't large enough to warrant it's own file, very edge case kinda thing, and usually it means that I should probably split'em out into a file that organizes similar functions. Hopefully recent edits now look closer to proper for future readers.





              That bit within the main_loop method with while self.keep_fishing(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 classy 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]['amount'] 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.






              share|improve this answer










              New contributor




              S0AndS0 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
              Check out our Code of Conduct.






              $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 they're classified under are "magic methods"


              • what I share is not gospel as such, but a collection of tricks I wish someone had shown me when I was getting into Python


              ... 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"

              #
              # Functions
              #

              def question(message):
              """ Returns response to `message` from user """
              return input("message? ".format(message = message))


              #
              # Classes
              #

              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 keep_fishing(message, expected):
              """ Return `bool`ean of if `response` to `message` matches `expected` """
              response = question(message)
              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(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("0nThanks 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)]).




              Note from the future; check the comments of this answer for some swell suggestions from @Izaak van Dongen.





              By defining a class that inherits from the built in dictionary 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.




              Side note; the __init__ method is one of many that are called implicitly by preforming some action with an object, eg. __add__ is called implicitly by using + between two Objects with a __add__ method (side-side note, I'll get into why that was an a and not an an in a bit), which is why the following works with lists...




              list_one = [3, 2, 1]
              list_two = [0, -1, -2]

              list_one + list_two
              # -> [3, 2, 1, 0, -1, -2]


              That bit with **kwargs stands for key word arguments which passes things as a bare dictionary, 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 other than saying that context matters. 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 dictionary, 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.




              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 propertys 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 staticmethods, 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 so operate similarly to regular functions, eg...



              responses = []

              responses.append(question("Where to"))
              print("I heard -> response".format(response = responses[-1]))
              for _ in range(7):
              responses.append(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 using f 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.




              Note from the future; as pointed out by @Maarten Fabré I indeed slipped in some superfluous use of the staticmethod decorator, good catch there, and this'll now serve as an example of getting carried away when decorating.



              Generally I use staticmethods when I've a class that isn't concerned with it's internal state but isn't large enough to warrant it's own file, very edge case kinda thing, and usually it means that I should probably split'em out into a file that organizes similar functions. Hopefully recent edits now look closer to proper for future readers.





              That bit within the main_loop method with while self.keep_fishing(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 classy 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]['amount'] 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.







              share|improve this answer










              New contributor




              S0AndS0 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
              Check out our Code of Conduct.









              share|improve this answer



              share|improve this answer








              edited 2 days ago





















              New contributor




              S0AndS0 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
              Check out our Code of Conduct.









              answered Apr 13 at 7:53









              S0AndS0S0AndS0

              3216




              3216




              New contributor




              S0AndS0 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
              Check out our Code of Conduct.





              New contributor





              S0AndS0 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
              Check out our Code of Conduct.






              S0AndS0 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
              Check out our Code of Conduct.







              • 5




                $begingroup$
                Welcome to Code Review! I think you did a grat job on your first answer here. Keep it going!
                $endgroup$
                – Alex
                Apr 13 at 8:01






              • 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
                Apr 13 at 23:06







              • 1




                $begingroup$
                print("amount fish".format(**'fish': fish, 'amount': data['amount'])) -- why not print("amount fish".format(fish=fish, amount=data['amount']))?
                $endgroup$
                – kevinsa5
                Apr 14 at 3:20






              • 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
                Apr 14 at 17:41






              • 2




                $begingroup$
                If you really want to use classes (do you?), I would expect a class for a Fish, a Pond, a Fisher and perhaps a Game. Not like this. All these staticmethods are unneeded on the class, but should go in the module namespace. You make a lot of simple things extra complicated. I suggest you post this piece of code as a seperate question, then you can get some tips on how to improve this.
                $endgroup$
                – Maarten Fabré
                Apr 15 at 8:59












              • 5




                $begingroup$
                Welcome to Code Review! I think you did a grat job on your first answer here. Keep it going!
                $endgroup$
                – Alex
                Apr 13 at 8:01






              • 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
                Apr 13 at 23:06







              • 1




                $begingroup$
                print("amount fish".format(**'fish': fish, 'amount': data['amount'])) -- why not print("amount fish".format(fish=fish, amount=data['amount']))?
                $endgroup$
                – kevinsa5
                Apr 14 at 3:20






              • 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
                Apr 14 at 17:41






              • 2




                $begingroup$
                If you really want to use classes (do you?), I would expect a class for a Fish, a Pond, a Fisher and perhaps a Game. Not like this. All these staticmethods are unneeded on the class, but should go in the module namespace. You make a lot of simple things extra complicated. I suggest you post this piece of code as a seperate question, then you can get some tips on how to improve this.
                $endgroup$
                – Maarten Fabré
                Apr 15 at 8:59







              5




              5




              $begingroup$
              Welcome to Code Review! I think you did a grat job on your first answer here. Keep it going!
              $endgroup$
              – Alex
              Apr 13 at 8:01




              $begingroup$
              Welcome to Code Review! I think you did a grat job on your first answer here. Keep it going!
              $endgroup$
              – Alex
              Apr 13 at 8:01




              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
              Apr 13 at 23:06





              $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
              Apr 13 at 23:06





              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
              Apr 14 at 3:20




              $begingroup$
              print("amount fish".format(**'fish': fish, 'amount': data['amount'])) -- why not print("amount fish".format(fish=fish, amount=data['amount']))?
              $endgroup$
              – kevinsa5
              Apr 14 at 3:20




              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
              Apr 14 at 17:41




              $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
              Apr 14 at 17:41




              2




              2




              $begingroup$
              If you really want to use classes (do you?), I would expect a class for a Fish, a Pond, a Fisher and perhaps a Game. Not like this. All these staticmethods are unneeded on the class, but should go in the module namespace. You make a lot of simple things extra complicated. I suggest you post this piece of code as a seperate question, then you can get some tips on how to improve this.
              $endgroup$
              – Maarten Fabré
              Apr 15 at 8:59




              $begingroup$
              If you really want to use classes (do you?), I would expect a class for a Fish, a Pond, a Fisher and perhaps a Game. Not like this. All these staticmethods are unneeded on the class, but should go in the module namespace. You make a lot of simple things extra complicated. I suggest you post this piece of code as a seperate question, then you can get some tips on how to improve this.
              $endgroup$
              – Maarten Fabré
              Apr 15 at 8:59











              10












              $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)





              share|improve this answer











              $endgroup$












              • $begingroup$
                For the behavior to be the same as the original code, nothing should have chance 3
                $endgroup$
                – Vaelus
                Apr 14 at 20:42










              • $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
                Apr 14 at 20:45






              • 1




                $begingroup$
                Whoops, you're right. Yet another reason to use your method.
                $endgroup$
                – Vaelus
                Apr 14 at 23:36















              10












              $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)





              share|improve this answer











              $endgroup$












              • $begingroup$
                For the behavior to be the same as the original code, nothing should have chance 3
                $endgroup$
                – Vaelus
                Apr 14 at 20:42










              • $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
                Apr 14 at 20:45






              • 1




                $begingroup$
                Whoops, you're right. Yet another reason to use your method.
                $endgroup$
                – Vaelus
                Apr 14 at 23:36













              10












              10








              10





              $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)





              share|improve this answer











              $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)






              share|improve this answer














              share|improve this answer



              share|improve this answer








              edited Apr 14 at 17:15

























              answered Apr 13 at 8:48









              GraipherGraipher

              27.6k54499




              27.6k54499











              • $begingroup$
                For the behavior to be the same as the original code, nothing should have chance 3
                $endgroup$
                – Vaelus
                Apr 14 at 20:42










              • $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
                Apr 14 at 20:45






              • 1




                $begingroup$
                Whoops, you're right. Yet another reason to use your method.
                $endgroup$
                – Vaelus
                Apr 14 at 23:36
















              • $begingroup$
                For the behavior to be the same as the original code, nothing should have chance 3
                $endgroup$
                – Vaelus
                Apr 14 at 20:42










              • $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
                Apr 14 at 20:45






              • 1




                $begingroup$
                Whoops, you're right. Yet another reason to use your method.
                $endgroup$
                – Vaelus
                Apr 14 at 23:36















              $begingroup$
              For the behavior to be the same as the original code, nothing should have chance 3
              $endgroup$
              – Vaelus
              Apr 14 at 20:42




              $begingroup$
              For the behavior to be the same as the original code, nothing should have chance 3
              $endgroup$
              – Vaelus
              Apr 14 at 20:42












              $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
              Apr 14 at 20:45




              $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
              Apr 14 at 20:45




              1




              1




              $begingroup$
              Whoops, you're right. Yet another reason to use your method.
              $endgroup$
              – Vaelus
              Apr 14 at 23:36




              $begingroup$
              Whoops, you're right. Yet another reason to use your method.
              $endgroup$
              – Vaelus
              Apr 14 at 23:36











              6












              $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))





              share|improve this answer











              $endgroup$

















                6












                $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))





                share|improve this answer











                $endgroup$















                  6












                  6








                  6





                  $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))





                  share|improve this answer











                  $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))






                  share|improve this answer














                  share|improve this answer



                  share|improve this answer








                  edited Apr 13 at 6:46

























                  answered Apr 13 at 6:31









                  VaelusVaelus

                  1714




                  1714




















                      Mattthecommie is a new contributor. Be nice, and check out our Code of Conduct.









                      draft saved

                      draft discarded


















                      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.




                      draft saved


                      draft discarded














                      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





















































                      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







                      Popular posts from this blog

                      Club Baloncesto Breogán Índice Historia | Pavillón | Nome | O Breogán na cultura popular | Xogadores | Adestradores | Presidentes | Palmarés | Historial | Líderes | Notas | Véxase tamén | Menú de navegacióncbbreogan.galCadroGuía oficial da ACB 2009-10, páxina 201Guía oficial ACB 1992, páxina 183. Editorial DB.É de 6.500 espectadores sentados axeitándose á última normativa"Estudiantes Junior, entre as mellores canteiras"o orixinalHemeroteca El Mundo Deportivo, 16 setembro de 1970, páxina 12Historia do BreogánAlfredo Pérez, o último canoneiroHistoria C.B. BreogánHemeroteca de El Mundo DeportivoJimmy Wright, norteamericano do Breogán deixará Lugo por ameazas de morteResultados de Breogán en 1986-87Resultados de Breogán en 1990-91Ficha de Velimir Perasović en acb.comResultados de Breogán en 1994-95Breogán arrasa al Barça. "El Mundo Deportivo", 27 de setembro de 1999, páxina 58CB Breogán - FC BarcelonaA FEB invita a participar nunha nova Liga EuropeaCharlie Bell na prensa estatalMáximos anotadores 2005Tempada 2005-06 : Tódolos Xogadores da Xornada""Non quero pensar nunha man negra, mais pregúntome que está a pasar""o orixinalRaúl López, orgulloso dos xogadores, presume da boa saúde económica do BreogánJulio González confirma que cesa como presidente del BreogánHomenaxe a Lisardo GómezA tempada do rexurdimento celesteEntrevista a Lisardo GómezEl COB dinamita el Pazo para forzar el quinto (69-73)Cafés Candelas, patrocinador del CB Breogán"Suso Lázare, novo presidente do Breogán"o orixinalCafés Candelas Breogán firma el mayor triunfo de la historiaEl Breogán realizará 17 homenajes por su cincuenta aniversario"O Breogán honra ao seu fundador e primeiro presidente"o orixinalMiguel Giao recibiu a homenaxe do PazoHomenaxe aos primeiros gladiadores celestesO home que nos amosa como ver o Breo co corazónTita Franco será homenaxeada polos #50anosdeBreoJulio Vila recibirá unha homenaxe in memoriam polos #50anosdeBreo"O Breogán homenaxeará aos seus aboados máis veteráns"Pechada ovación a «Capi» Sanmartín e Ricardo «Corazón de González»Homenaxe por décadas de informaciónPaco García volve ao Pazo con motivo do 50 aniversario"Resultados y clasificaciones""O Cafés Candelas Breogán, campión da Copa Princesa""O Cafés Candelas Breogán, equipo ACB"C.B. Breogán"Proxecto social"o orixinal"Centros asociados"o orixinalFicha en imdb.comMario Camus trata la recuperación del amor en 'La vieja música', su última película"Páxina web oficial""Club Baloncesto Breogán""C. B. Breogán S.A.D."eehttp://www.fegaba.com

                      Vilaño, A Laracha Índice Patrimonio | Lugares e parroquias | Véxase tamén | Menú de navegación43°14′52″N 8°36′03″O / 43.24775, -8.60070

                      Cegueira Índice Epidemioloxía | Deficiencia visual | Tipos de cegueira | Principais causas de cegueira | Tratamento | Técnicas de adaptación e axudas | Vida dos cegos | Primeiros auxilios | Crenzas respecto das persoas cegas | Crenzas das persoas cegas | O neno deficiente visual | Aspectos psicolóxicos da cegueira | Notas | Véxase tamén | Menú de navegación54.054.154.436928256blindnessDicionario da Real Academia GalegaPortal das Palabras"International Standards: Visual Standards — Aspects and Ranges of Vision Loss with Emphasis on Population Surveys.""Visual impairment and blindness""Presentan un plan para previr a cegueira"o orixinalACCDV Associació Catalana de Cecs i Disminuïts Visuals - PMFTrachoma"Effect of gene therapy on visual function in Leber's congenital amaurosis"1844137110.1056/NEJMoa0802268Cans guía - os mellores amigos dos cegosArquivadoEscola de cans guía para cegos en Mortágua, PortugalArquivado"Tecnología para ciegos y deficientes visuales. Recopilación de recursos gratuitos en la Red""Colorino""‘COL.diesis’, escuchar los sonidos del color""COL.diesis: Transforming Colour into Melody and Implementing the Result in a Colour Sensor Device"o orixinal"Sistema de desarrollo de sinestesia color-sonido para invidentes utilizando un protocolo de audio""Enseñanza táctil - geometría y color. Juegos didácticos para niños ciegos y videntes""Sistema Constanz"L'ocupació laboral dels cecs a l'Estat espanyol està pràcticament equiparada a la de les persones amb visió, entrevista amb Pedro ZuritaONCE (Organización Nacional de Cegos de España)Prevención da cegueiraDescrición de deficiencias visuais (Disc@pnet)Braillín, un boneco atractivo para calquera neno, con ou sen discapacidade, que permite familiarizarse co sistema de escritura e lectura brailleAxudas Técnicas36838ID00897494007150-90057129528256DOID:1432HP:0000618D001766C10.597.751.941.162C97109C0155020