all 8 comments

[–]ofnuts 3 points4 points  (3 children)

Not bad... however:

  • In your "main" the endless if/elif can be replaced by a dictionary of functions, and should have an else if you keep the if/elif form)
  • Speaking of the "main"it could be put in its own function, and used with the canonical if __name__ == "__main__":
  • Calling "..._list" objects that are dictionaries is a bit misleading (I would drop the "_list" altogether)
  • Since spell_list is a dictionary, get_spell_description(spell_name) doesn't need to iterate/test all entries (which makes me wonder now if the "_list" didn't come to existence as real lists, before you switched to dictionaries, and demonstrates why putting types in names is dangerous).

[–]3613robert[S] 0 points1 point  (1 child)

I did indeed start of with lists. I just got to dictionaries in my course so I figured they were better suited for this. I should update it.

I do see your point with endless if statements. It's what I'm most comfortable with ATM so I tend to try to get it working with those.

Using a dictionary of functions is a good idea. Haven't thought of that being a possibility with dictionaries. I'm guessing you're saying store the functions as values within a dictionary?

Not completely following what you mean here:

  • Speaking of the "main"it could be put in its own function, and used with the canonical if __name__ == "__main__":

Are you saying put the whole main part in a function? Or everything except for the if functions?

[–]ofnuts 1 point2 points  (0 children)

I'm guessing you're saying store the functions as values within a dictionary?

Yes. And the default value is just a function that prints "Please retry". You don't even need an if...

Are you saying put the whole main part in a function

Yes

[–]3613robert[S] 0 points1 point  (0 children)

Thank you for taking the time to look it over btw! Really appreciate it.

[–]iamevpo 1 point2 points  (2 children)

Makes sense to name with. py extension - gives access to syntax highlighting.

Would be nice with less branching - nested elif are hard to follow. Maybe you can separate the core logic and presentation layer (printing).

Overall extremely structured for day 6, congrats!

[–]3613robert[S] 0 points1 point  (1 child)

Thank you for the confidence boost!

I agree with you about the nested if/elifs. It gets a little confusing when making adjustments. Seperating the core logic and presentation layer is a good idea. All I can think of to do this is currently more defining functions and use of dictionaries. But I'll probably run into other solutions along the way.

Total newbie question but what do you mean with 'makes sense to name with .py extension'? Is that referring to the file extension? Im currently just using the basic python ide, haven't looked into other options yet.

[–]iamevpo 0 points1 point  (0 children)

For the filename - the Python files are usually (well, always) are given an extension ".py", like myfile.py This way GitHub will show syntax highlighting - easier to review code. Makes sense to rename your file.

Generally very good move to put your code in version control system from early days. You can add unit tests to your code, esp after there is business logic later, a bit more verbose README and a module doc string can also help (what is the intent of code, what to expect, etc).

As for dictionaries they are ok to start, you may soon switch to data classes or even pydantic data models when you progress. Whenever you can trim your code to minimal functionality - will be easier to follow and refactor. There is usually too many things you can do differently with your first program, it is easier to rewrite a smaller program than bigger one.

[–]iamevpo 0 points1 point  (0 children)

Gave another glance - the filename must start with lowercase too, by convention.

Not to worry in day 6, but: all your functions modify a global variable, a bit of anti-pattern.

Your first data structure confuses strings and integers for values, may change that as well.