Feedback on part 12

[DataBeaver]

I went through this tutorial because a friend was asking for my opinions on it. There's a few things I'd have done differently, but for the most part nothing stood out as actually wrong. However part 12 has a couple things worthy of comment.

You use simple lists of 2-tuples for maximum item and monster counts, but dictionaries for spawn chances. Yet you only ever iterate through the dictionary like it was a list - there's no lookups by key and the data is static so the unique-key constraint is not utilized either. My instincts say that you should pick the simplest container which provides all the operations you need at an acceptable complexity, so the spawn chances should also be a list. In fact, using a list would allow using the bisect module from Python's standard library to quickly locate the most appropriate entry. With such small numbers of items this is of course rather inconsequential, but it could be an interesting side lesson about data structures and algorithms.

You're also passing the floor number to place_entities and calling get_max_value_for_floor from there. This looks up the max values again for every room, even though the results are the same every time. I would have preserved the parameters to place_entities and done the lookups in generate_dungeon instead, before the rooms loop.

[HexDecimal]

I agree that this could be better, but it's not clear if the data should be preprocessed by a function or if a new class should be made to encapsulate the data.

Did you have any code examples on your own project or is this just speculation so far?

[DataBeaver]

I haven't actually written any code based on the tutorial; this is just based on my thoughts from reading it. In the simplest case the data is entered into the source code already in the correct format. To reduce the risk of human error, I think it would make sense to create a class to hold the configuration data and sort it in the constructor (or after loading it from a file or whatever way it is that the data gets into the class).

I could do a quick fork and create a pull request if you think that would be useful.

[HexDecimal]

I'm not sure. If you want to do a pull request then make sure you start from the part-12 branch.

Ideally a new solution should be more readable then the current one. The performance for this section is not critical so I don't think the redundant calls to get_max_value_for_floor are a problem unless they affect readability.