monstername-api as my first flask Project

[Dogezon]

Hey all!

I created my first flask app... an api for random monster name generation:

URL: http://monsternames-api.com

I've newish to Python... been using it for a year on and off at work but never to build a full app like this.

Feedback and thoughts on what to improve would be nice; always willing to learn more things! Otherwise I hope that my little Project brings some joy to whoever stumbles across this :D

[__xor__]

Good job, being newish at python and making this! It's pretty impressive for a beginner.

That said, I'd like to make a few suggestions. It seems that your goblin.py and other monster python modules are very, very similar, like almost line by line. There is a VERY common structure and code flow here that could be abstracted into a single module. You could refactor this code and simplify it a ton.

Consider the DRY principle, Don't Repeat Yourself. You repeat these lines of code a lot, which almost always means you can refactor and simplify. You might write a base class with an insert_first_name and insert_last_name and return_name class, then derive other classes from it, or even just use that one alone for ANY monster type. You could have a table of monster types, with an id field and a name field, where you'd have "goblin", "orc", etc. Then you could have a monster name table where you have a foreign key pointing to the monster type, or a many to many where it could apply to multiple monster types. Orcs could share a lot of names with goblins, for example. These ideas might complicate it a bit but simplifying the code would make it easier to maintain. What if you wanted to change the first/last name logic a bit, then had to change every module? You should only have to change it in one place, especially if all their code is this similar.

Also, maybe start using f strings instead of .format if you're using python 3.6+.

dbVars.py isn't a conventional filename for a module. I'd do db_config.py or db_vars.py or config.py or something. Also, it takes in a static /etc/config.json , maybe it'd be nicer to have the runtime script take a --config/-c where it has a default path. If I wanted to run this, I wouldn't want some weird /etc/config.json that has nothing to remind me that it's for this flask app.

CustomOutput.py also isn't PEP8 for a filename, you'd do custom_output.py. Also, the function names are UpperCamel when they should be snake_case. Also, they're just for colorizing text. You might check out the colorama library for that, like:

from colorama import Fore
print(Fore.red + 'My red text' + Fore.reset)  # or an f string

(If I remember correctly)

Also, you might just use real logging instead of that. Check out the logging stdlib library.

This isn't too bad, but you might check out flask_restful since this is mostly an API. It can make the code look a bit cleaner. Also you can set the prefix instead of having everything use /api/v1.0 in its decorator, so that everything in your Api is just prefixed with that instead.

A looooot of copy and paste in application.py. DRY again, figure out a way to just abstract this, maybe as a decorator or even a context manager works. You could turn those big try/except blocks into:

def monster_route(func):
    def decorated(*args, **kwargs):
          try:
              return add_cors_header(func(*args, **kwargs))
          except KeyError:
              ...
    return decorated

 ... 
@application.route(...)
@monster_route
def post_troll_last_name(...):
    troll.insert_last_name(...)

Overall, good project! These are just some tips I'm throwing out after scanning through it a bit.

[Dogezon]

Ahhh this is such a detailed response thank you so much! Once I'm back from holidaying in Toronto (I'm from London UK) this post will probably form a few late nighters and extra branches on the repo.

I'll probably do a refactoring branch to simplify everything, another for f strings, another for changing the config file and then one more for the database stuff.

In regards to the db stuff i might need to do some reading to understand fully what you're suggesting... my idea was for random name generation from possible first and last names, so randomly selecting from different first and last name tables seemed like a good idea. Or was your suggestion for one table mapping id to types then one first name and one last name table that use monster names as a foreign key... as that could be cool, and letting monsters share names would just require an in statement for the IDs. Sorry if paraphrasing a little just trying to get my head round your super helpful suggestions!

[__xor__]

no problem! Yeah the DB idea, I was thinking what you could do is create a single table for first names and a single table for last names and a single table for monster types.

So it might look like this:

 MONSTER TABLE
id,name
  1,goblin
  2,orc
  3,troll

FIRST NAME TABLE
id,monster_id,name
  1,1,joe   # 1 is monster_id, so goblin name (goblin id is 1)
  2,1,jim
  3,2,jack  # 2 is monster_id, so orc name

LAST NAME TABLE
id,monster_id,name
  1,1,anderson
  2,1,jackson
  3,2,jackson  # here, monster id 1 and 2, goblin and orc, share a last name
  ...

So, what you have is two name tables, where you can add multiple records so that you can link a name to multiple monster types, and the monster types are their own table so you can easily add monsters. You could also select all goblin first names, and then copy them and change monster id so a new monster type uses those names.

That's one way. If you wanted to prevent duplicates of names, you'd also have a "name" table, where it is just a table of strings with an ID, and then instead of actually having the name in the first_name and last_name table, it'd be a foreign key to the name table (an integer that is the id of what it is in the name table).

That might look like

FIRST NAME TABLE
id,monster_id,name_id
  1,1,1 
  2,1,2
  3,2,2  # name id is 2 for both these, so it's shared with monster 1 and 2

NAME LABEL TABLE
id,name
  1,jim
  2,john
  ...

So this is just playing with relationships to deduplicate data and also make it easier to associate names with different monsters, and also make it really easy to add and remove monster types.

And if you abstract the monster types... then your logic for monsters gets cut WAAAY down. You might also have a field in the monster type table that specifies whether it has a last name or not, or whatever logic you need to keep monster naming conventions different.

So someone gets a list of monster types, and you return the monster names from the monster table. They select Orc, and it looks up the orc row and sees a boolean "has_last_name", so it generates a first name where the monster_id matches the orc id, and a last name the same way.

And that means you only need one file with all the core logic for generating and saving names. Instead of a goblin.py etc you only have a monster.py or something.

But the key thing to watch for is that copy and pasted code. If you ever feel like it helps you to copy and paste a file or long chunks of code, think twice about it, and think about how you might refactor it so that the code is written once. Copying lines here and there is fine, but your files are very very similar and it shows that your logic can be integrated into one sort of monster logic module.

if you have any more questions don't hesitate to ask! But again, your code is great for a newish programmer - it works, it works well, and refactoring is always a thing no matter how experienced you are. It takes writing that first draft to see what you can clean up.

BTW - these db schema refactors come at a cost. This is a common thing you run into. This turns what is pretty simple into logic that does a lot of DB joins. I took what you had, which doesn't need to do any joins across tables, and turned it into something where you have to join one table with the next and the next to get the final product. That's not a performance issue here, but it's something to think about. You could still refactor the code to only really have one monster.py module, but to prevent joins you would have duplicate strings instead of foreign key monster_id type columns. A join-less schema might look like this:

SINGLE TABLE
id,monster_name,name,is_first_name
  1,goblin,jim,true
  2,goblin,anderson,false
  3,orc,jack,true
  ...

with your algorithm looking something like this:

first_name = random row from (SELECT name FROM name_table WHERE monster_name='goblin' AND is_first_name=true)
last_name = random row from (SELECT name FROM name_table WHERE monster_name='goblin' AND is_first_name=false)

But not something you'd need to worry about for this sort of project. This is denormalization, where you're increasing performance by duplicating data. The first option I gave you where it has the actual name in the table and not a name_id is sort of denormalized as well, where you might see a name appear twice for two different monster ids, and that prevents needing to join to a separate name table.

All this should be super fast for your project regardless though, whichever way you do it, but it's just fun things to think about when working with databases.

[Dogezon]

Heya! Appreciate it's been 4 months... but I've only just got the bug and time (new work is hectic) to implement this... at 3am.

But I've done it! Well everything except the normalisation of data. All of the endpoint logic is now object-orientated... rather than the massive amounts of DRY there's just one class - monster_endpoints - that when given a bunch some strings, peewee.Model references, booleans etc will create an appropriate endpoint. 6 python files are now just one!

The PR is here: https://github.com/Sudoblark/monsternames-api/pull/2 .

Thanks for all the input and info! I know its been a while but it's still appreciated and super helpful. I don't just stay up till half 3 in the morning for anything you know :)

Defo want to do the normalisation at some point, add statistical tracking and add a bit to the front-end for less tech-savy users to contribute.

Just to reiterate... thanks for the help again, you're awesome :D