Python discord bot

Question

So in my free time, I made a discord bot to play around with and learn a bit of python. I'm mainly a java developer. The bot has a few commands that allow it to find and output recipe for cocktails and a few other things revolving around cocktails.

In the background, there are 2 JSON files. My bot mainly reads and filters these JSON files and returns what it finds. In java, I am used to streams which I don't have in python so I just had to make do with what I know but I get a feeling that some things can definitely be done cleaner/shorter/better/ and I'm not sure everything follows python standards so I would be grateful for any feedback.

---

 [ !! cocktails can be exchanged with -c, ingredients with -i !! ] [ <KW> find cocktails <search criteria1> <search criteria2> ... Find cocktail. ] [ <KW> find ingredients <search criteria1> <search criteria2> ... Find ingredient. ] [ <KW> cocktails List all cocktails. ] [ <KW> ingredients List all ingredients. ] [ <KW> categories List all cocktail categories. ] [ <KW> list <category> List all cocktails by category ] [ <KW> count cocktails How many cocktails? ] [ <KW> count ingredients How many ingredients? ] [ <KW> with <ingredient> <ingredient2> ... Cocktails with ingredient. ] [ <KW> recipe <cocktail> How to make a cocktail. ] 

The possible commands are as above <KW> is the trigger word for the bot that a different component handles. bartender.py receives the content with this cut off.

---

bartender.py

import json class Bartender: with open("recipes.json") as recipes_file: recipes = json.load(recipes_file) with open("ingredients.json") as ingredients_file: ingredients = json.load(ingredients_file) default = "This command does not exist or you mistyped something" error = "There was a problem with processing the command" def handle(self, message): command_prefix = message.content.strip().lower() answer = self.default if self.starts_with_ingredients_prefix(command_prefix): answer = self.get_all_ingredients() elif self.starts_with_cocktails_prefix(command_prefix): answer = self.get_all_cocktails() elif command_prefix == "categories": answer = self.get_all_categories() elif command_prefix.startswith("count"): answer = self.get_count(command_prefix.removeprefix("count").strip()) elif command_prefix.startswith("recipe"): answer = self.get_recipe(command_prefix.removeprefix("recipe").strip()) elif command_prefix.startswith("list"): answer = self.get_cocktails_by_category(command_prefix.removeprefix("list").strip()) elif command_prefix.startswith("find"): answer = self.find(command_prefix.removeprefix("find").strip()) elif command_prefix.startswith("with"): answer = self.get_cocktails_with(command_prefix.removeprefix("with").strip()) return answer def get_all_ingredients(self): """Returns a string containing all of the ingredients the bot knows.""" answer = "" for key, value in self.ingredients.items(): answer += f"{key} ({value.get('abv')}%)\n" return answer def get_all_cocktails(self): """Returns a string containing the names of all of the cocktails the bot knows.""" answer = "" for cocktail in self.recipes: answer += f"{cocktail.get('name')}\n" return answer def get_all_categories(self): """Returns a string containing all the cocktail categories the bot knows.""" answer = "" categories_list = [] for cocktail in self.recipes: categories_list.append(cocktail.get("category")) # Remove duplicates categories_list = list(set(categories_list)) categories_list.sort(key=str) for category in categories_list: answer += f"{category}\n" return answer def get_count(self, param): """Returns the amount of ingredients or cocktails the bot knows.""" answer = self.error if self.starts_with_ingredients_prefix(param): answer = len(self.ingredients) elif self.starts_with_cocktails_prefix(param): answer = len(self.recipes) return answer def get_recipe(self, param): """Returns the full recipe for the passed cocktail name.""" answer = f"There is no recipe for a cocktail called {param}. To see all cocktails with a recipe " \ f"type '$bt cocktails'" for cocktail in self.recipes: if param == cocktail.get("name").lower(): formatted_ingredients = self.get_formatted_ingredients(cocktail.get("ingredients")) garnish = self.get_garnisch(cocktail) return f"__**{cocktail.get('name')}**__\n" \ f"**Ingriedients:**\n" \ f"{formatted_ingredients}" \ f"{garnish}" \ f"**Preparation:**\n" \ f"{cocktail.get('preparation')} \n" return answer def get_formatted_ingredients(self, ingredients): """Returns a string of ingredients formatted as list for the cocktails including the special ones if it has any.""" formatted_ingredients = "" special_ingredients = "" for ingredient in ingredients: if ingredient.get("special") is not None: special_ingredients += f" - {ingredient.get('special')}\n" else: formatted_ingredients += f" - {ingredient.get('amount')} {ingredient.get('unit')} {ingredient.get('ingredient')} " if ingredient.get("label") is not None: f"({ingredient.get('label')})" formatted_ingredients += "\n" return formatted_ingredients + special_ingredients def get_garnisch(self, cocktail): """Returns the garnish for the cocktail if it has one.""" if cocktail.get("garnish") is not None: return f"**Garnish:**\n" \ f" - {cocktail.get('garnish')} \n" else: return "" def get_cocktails_by_category(self, category): """Returns all cocktails in the given category.""" answer = "" for cocktail in self.recipes: if category == str(cocktail.get("category")).lower(): answer += f"{cocktail.get('name')}\n" return answer if len(answer) > 0 else f"There is no category called {category} or it contains no cocktails" def starts_with_cocktails_prefix(self, param): """Returns true if passed string starts with the cocktails prefix (-c or cocktails).""" return param.startswith("-c") or param.startswith("cocktails") def remove_cocktails_prefix(self, param): """Returns a string with the cocktails prefix (-c or cocktails) removed. If the string does not start with the cocktails prefix it will return the original string.""" if param.startswith("-c"): param = param.removeprefix("-c") elif param.startswith("cocktails"): param = param.removeprefix("cocktails") return param def starts_with_ingredients_prefix(self, param): """Returns true if passed string starts with the ingredient prefix (-i or ingredients).""" return param.startswith("-i") or param.startswith("ingredients") def remove_ingredients_prefix(self, param): """Returns a string with the ingredient prefix (-i or ingredients) removed. If the string does not start with the ingredients prefix it will return the original string.""" if param.startswith("-i"): param = param.removeprefix("-i") elif param.startswith("ingredients"): param = param.removeprefix("ingredients") return param def find(self, param): """Returns all ingredients or cocktails containing the criteria in the parameter separated by commas.""" answer = "" if self.starts_with_cocktails_prefix(param): param = self.remove_cocktails_prefix(param) for criteria in param.strip().split(): answer += f"**Criteria: {criteria}**\n" answer += self.get_cocktails_containing(criteria) elif self.starts_with_ingredients_prefix(param): param = self.remove_ingredients_prefix(param) for criteria in param.strip().split(): answer += f"**Criteria: {criteria}**\n" answer += self.get_ingredients_containing(criteria) return answer if len(answer) > 0 else "Nothing was found matching your criteria" def get_cocktails_containing(self, criteria): """Returns all cocktails containing the criteria in its name.""" answer = "" for cocktail in self.recipes: if criteria in str(cocktail.get("name")).lower(): answer += f"{cocktail.get('name')}\n" return answer if len(answer) > 0 else "Nothing was found matching your criteria" def get_ingredients_containing(self, criteria): """Returns all ingredients containing the criteria in its name.""" answer = "" for ingredient in self.ingredients.keys(): if criteria in ingredient.lower(): answer += f"{ingredient}\n" return answer if len(answer) > 0 else "Nothing was found matching your criteria" def get_cocktails_with(self, param): """Returns all cocktails containing the searched for ingredients in the parameter separated by commas.""" answer = "" for ingredient in param.strip().split(","): for cocktail in self.recipes: cocktail_ingredients = cocktail.get("ingredients") answer += self.does_cocktail_contain(cocktail, cocktail_ingredients, ingredient.strip()) return answer if len(answer) > 0 else "Nothing was found matching your criteria" def does_cocktail_contain(self, cocktail, cocktail_ingredients, ingredient): """Returns the name of the cocktail if the cocktail contains the searched for ingredient.""" for cocktail_ingredient in cocktail_ingredients: if cocktail_ingredient.get("ingredient") is not None and ingredient in cocktail_ingredient.get( "ingredient").lower(): return f"{cocktail.get('name')}\n" return "" 

---

recipes.json

[ { "name": "Vesper", "glass": "martini", "category": "Before Dinner Cocktail", "ingredients": [ { "unit": "cl", "amount": 6, "ingredient": "Gin" }, { "unit": "cl", "amount": 1.5, "ingredient": "Vodka" }, { "unit": "cl", "amount": 0.75, "ingredient": "Lillet Blonde" }, { "special": "3 dashes Strawberry syrup" } ], "garnish": "Lemon twist", "preparation": "Shake and strain into a chilled cocktail glass." }, ... ] 

Not all cocktails have the glass attribute and a cocktail can have 0 to n "special" ingredients.

---

ingredients.json

{ "Absinthe": { "abv": 40, "taste": null }, "Aperol": { "abv": 11, "taste": "bitter" }, ... } 

Answer 1

get_all_ingredients

List comprehensions are very pythonic and often times faster (especially for operations, that aren't computationally expensive), so instead of

answer = "" for key, value in self.ingredients.items(): answer += f"{key} ({value.get('abv')}%)\n" 

you should use

answer = "".join(f"{key} ({value.get('abv')}%)\n" for key, value in self.ingredients.items()) return answer 

or as a one-liner

return "".join(f"{key} ({value.get('abv')}%)\n" for key, value in self.ingredients.items()) 

str.join(...) joins each element of an iterable by a string separator (the string on which it is called) and returns the concatenated string.

What we're using here is actually not a list comprehension, but a generator expression. If you're interested you can read more about Generator Expressions vs List Comprehensions.

The same is also applicable to get_all_cocktails.

---

get_all_categories

As seen before we can replace

categories_list = [] for cocktail in self.recipes: categories_list.append(cocktail.get("category")) 

by a list comprehension. But since we actually want a sorted list of distinct values we can use a set comprehension or use set() and a generator expression.

{cocktail.get("category") for cocktail in self.recipes} # or set(cocktail.get("category") for cocktail in self.recipes) 

Now we wrap this with sorted() which returns a sorted list and we get categories_list concisely. Providing str as the key function should not be necessary since the values are already strings.

categories_list = sorted(set(cocktail.get("category") for cocktail in self.recipes)) 

Using str.join() again brings the entire method down to 2 lines:

categories_list = sorted(set(cocktail.get("category") for cocktail in self.recipes)) return "".join(f"{category}\n" for category in categories_list) 

Note that we don't need to assign an empty string as a default value since "".join(...) will return an empty string if there are no elements in the iterable.

---

get_formatted_ingredients

You will sometimes be able to replace

if ingredient.get("special") is not None: 

by

if ingredient.get("special"): 

This however depends on the use case, since the expressions are not equivalent. The second one will fail for all falsey values, not just None (e.g. an empty string in this case, which you also might want to skip). If you're interested, you can read more about Truthy and Falsy Values in Python.

Using the walrus operator we can make this more concise, and remove the redundant get:

if special_ingredient := ingredient.get("special") is not None: special_ingredients += f" - {special_ingredient}\n" 

I don't think the following code does anything at all, since it doesn't do anything with the string:

if ingredient.get("label") is not None: f"({ingredient.get('label')})" 

---

These are just some things I noticed. You will find these patterns in different places throughout your code. This is of course not a complete list of possible improvements, just the ones I found on my first read through your code.

Answer 2

Instance-ness

Your file reading is done in class static scope. It should be moved to class instance scope.

Type hints

Add some PEP484 type hints to your function signatures.

Type soundness and dictionary soup

Once you've read in your data from JSON, you should not leave it as dictionaries - make actual classes so that your data have stronger correctness guarantees and static analysis tools can actually say something about your code.

Another benefit of this approach is that your new classes for recipes and ingredients can have methods dealing with data specific to that class.

Indexing

You haven't built up any index dictionaries for faster, simpler lookup based on search terms; you should.

Line continuation

For long string, prefer to put them in parenthesized expressions rather than adding backslashes.

Bug: label

This statement:

f"({ingredient.get('label')})" 

does nothing, so you'll never see your label.

Spelling

garnisch -> garnish

Ingriedients -> Ingredients

Documentation lies

 """Returns all ingredients or cocktails containing the criteria in the parameter separated by commas.""" 

is not, in fact, what's happening. Your split() splits on whitespace.

Statics

Any method that doesn't actually touch the self reference doesn't need to be an instance method and can benefit from being a static, or sometimes a classmethod if (in simple terms) it references other statics or the constructor. In reality @classmethods can reference any attribute on the class object which is a touch more detail than I suspect you need right now.

Benefits to proper use of static methods include easier testability due to fewer internal dependencies, and more accurate representation of "what's actually happening" to other developers reading your code.

It's also worth mentioning (due to your comment) that public/private and instance/static are very different mechanisms. public/private, in other languages, controls visibility from the outside of the class, whereas instance/static controls whether the method is visible on an instance or on the class type itself. Python does not have a strong concept of public/private at all, aside from the leading-underscore-implying-private and double-underscore-implying-name-mangling conventions; neither is called for here I think.

Suggested

There are many ways to implement some of the above, particularly conversion to classes. Presented below is one way.

import json from collections import defaultdict from dataclasses import dataclass from itertools import chain from typing import Optional, Iterable, Tuple, Dict, Any, List @dataclass class FakeMessage: content: str @dataclass class Ingredient: name: str abv: float taste: Optional[str] @classmethod def from_json(cls, fname: str = 'ingredients.json') -> Iterable['Ingredient']: with open(fname) as ingredients_file: data = json.load(ingredients_file) for name, attributes in data.items(): yield cls(name=name, **attributes) def __str__(self): return f'{self.name} ({self.abv}%)' @dataclass class RecipeIngredient: unit: str amount: float ingredient: Ingredient label: Optional[str] = None @classmethod def from_dicts( cls, dicts: Iterable[Dict[str, Any]], all_ingredients: Dict[str, Ingredient], ) -> Iterable['RecipeIngredient']: for data in dicts: if 'special' not in data: name = data.pop('ingredient') yield cls(**data, ingredient=all_ingredients[name.lower()]) def __str__(self): desc = f'{self.amount} {self.unit} {self.ingredient}' if self.label is not None: desc += f' {self.label}' return desc @dataclass class Recipe: name: str glass: str category: str preparation: str ingredients: Tuple[RecipeIngredient, ...] special_ingredients: Tuple[str, ...] garnish: Optional[str] = None @classmethod def from_json( cls, all_ingredients: Dict[str, Ingredient], fname: str = 'recipes.json', ) -> Iterable['Recipe']: with open(fname) as recipes_file: data = json.load(recipes_file) for recipe in data: ingredient_data = recipe.pop('ingredients') specials = tuple( ingredient['special'] for ingredient in ingredient_data if 'special' in ingredient ) yield cls( **recipe, ingredients=tuple( RecipeIngredient.from_dicts(ingredient_data, all_ingredients) ), special_ingredients=specials, ) @property def formatted_ingredients(self) -> str: """Returns a string of ingredients formatted as list for the cocktails including the special ones if it has any.""" ingredients = chain(self.ingredients, self.special_ingredients) return '\n'.join( f' - {ingredient}' for ingredient in ingredients ) @property def as_markdown(self) -> str: desc = ( f"__**{self.name}**__\n" f"**Ingredients:**\n" f"{self.formatted_ingredients}\n" ) if self.garnish is not None: desc += f' - {self.garnish}\n' desc += ( f"**Preparation:**\n" f"{self.preparation}\n" ) return desc class Bartender: ERROR = "There was a problem with processing the command" def __init__(self): self.ingredients: Dict[str, Ingredient] = { i.name.lower(): i for i in Ingredient.from_json() } self.recipes: Dict[str, Recipe] = { r.name.lower(): r for r in Recipe.from_json(self.ingredients) } self.categories: Dict[str, List[Recipe]] = defaultdict(list) for recipe in self.recipes.values(): self.categories[recipe.category.lower()].append(recipe) self.by_ingredient: Dict[str, List[Recipe]] = defaultdict(list) for recipe in self.recipes.values(): for ingredient in recipe.ingredients: self.by_ingredient[ingredient.ingredient.name.lower()].append(recipe) def handle(self, message: FakeMessage) -> str: command_prefix = message.content.strip().lower() if self.starts_with_ingredients_prefix(command_prefix): return self.get_all_ingredients() if self.starts_with_cocktails_prefix(command_prefix): return self.get_all_cocktails() if command_prefix == "categories": return self.get_all_categories() if command_prefix.startswith("count"): return self.get_count(command_prefix.removeprefix("count").strip()) if command_prefix.startswith("recipe"): return self.get_recipe(command_prefix.removeprefix("recipe").strip()) if command_prefix.startswith("list"): return self.get_cocktails_by_category(command_prefix.removeprefix("list").strip()) if command_prefix.startswith("find"): return self.find(command_prefix.removeprefix("find").strip()) if command_prefix.startswith("with"): return self.get_cocktails_with(command_prefix.removeprefix("with").strip()) return "This command does not exist or you mistyped something" def get_all_ingredients(self) -> str: """Returns a string containing all of the ingredients the bot knows.""" return '\n'.join(str(i) for i in self.ingredients.values()) def get_all_cocktails(self) -> str: """Returns a string containing the names of all of the cocktails the bot knows.""" return '\n'.join(r.name for r in self.recipes.values()) def get_all_categories(self) -> str: """Returns a string containing all the cocktail categories the bot knows.""" categories = sorted({ recipe.category for recipe in self.recipes.values() }) return '\n'.join(categories) def get_count(self, param: str) -> str: """Returns the amount of ingredients or cocktails the bot knows.""" if self.starts_with_ingredients_prefix(param): sequence = self.ingredients elif self.starts_with_cocktails_prefix(param): sequence = self.recipes else: return self.ERROR return str(len(sequence)) def get_recipe(self, param: str) -> str: """Returns the full recipe for the passed cocktail name.""" recipe = self.recipes.get(param) if recipe is None: return ( f"There is no recipe for a cocktail called {param}. To see all " f"cocktails with a recipe type '$bt cocktails'" ) return recipe.as_markdown def get_cocktails_by_category(self, category: str) -> str: """Returns all cocktails in the given category.""" recipes = self.categories.get(category) if not recipes: return f"There is no category called {category} or it contains no cocktails" return '\n'.join(r.name for r in recipes) @staticmethod def starts_with_cocktails_prefix(param: str) -> bool: """Returns true if passed string starts with the cocktails prefix (-c or cocktails).""" return param.startswith("-c") or param.startswith("cocktails") @staticmethod def remove_cocktails_prefix(param: str) -> str: """Returns a string with the cocktails prefix (-c or cocktails) removed. If the string does not start with the cocktails prefix it will return the original string.""" if param.startswith("-c"): return param.removeprefix("-c") if param.startswith("cocktails"): return param.removeprefix("cocktails") return param @staticmethod def starts_with_ingredients_prefix(param: str) -> bool: """Returns true if passed string starts with the ingredient prefix (-i or ingredients).""" return param.startswith("-i") or param.startswith("ingredients") @staticmethod def remove_ingredients_prefix(param: str) -> str: """Returns a string with the ingredient prefix (-i or ingredients) removed. If the string does not start with the ingredients prefix it will return the original string.""" if param.startswith("-i"): return param.removeprefix("-i") if param.startswith("ingredients"): return param.removeprefix("ingredients") return param def find(self, param: str) -> str: """Returns all ingredients or cocktails containing the criteria in the parameter separated by commas.""" answer = "" if self.starts_with_cocktails_prefix(param): param = self.remove_cocktails_prefix(param) for criteria in param.strip().split(): answer += f"**Criteria: {criteria}**\n" answer += self.get_cocktails_containing(criteria) return answer if self.starts_with_ingredients_prefix(param): param = self.remove_ingredients_prefix(param) for criteria in param.strip().split(): answer += f"**Criteria: {criteria}**\n" answer += self.get_ingredients_containing(criteria) return answer return self.ERROR def get_cocktails_containing(self, criteria: str) -> str: """Returns all cocktails containing the criteria in its name.""" answer = "" for name, recipe in self.recipes.items(): if criteria in name: answer += f"{recipe.name}\n" if answer: return answer return "Nothing was found matching your criteria" def get_ingredients_containing(self, criteria: str) -> str: """Returns all ingredients containing the criteria in its name.""" answer = "" for name, ingredient in self.ingredients.items(): if criteria in name: answer += f"{ingredient.name}\n" if answer: return answer return "Nothing was found matching your criteria" def get_cocktails_with(self, param: str) -> str: """Returns all cocktails containing the searched for ingredients in the parameter separated by commas.""" answer = '\n'.join( recipe.name for ingredient in param.strip().split(",") for recipe in self.by_ingredient.get(ingredient, ()) ) if answer: return answer return "Nothing was found matching your criteria" def test(): bar = Bartender() while True: try: cmd = input('> ') except KeyboardInterrupt: break print(bar.handle(FakeMessage(cmd))) print() test() 

Answer 3

You could replace some for loops with list comprehensions. I think list comprehensions are generally preferred and often faster. I've also:

• added a default value to the get command in case your JSON is malformed or missing the ABV for some reason, and
• replaced the \n on the end of each line with the join method.

def get_all_ingredients(self): """Returns a string containing all of the ingredients the bot knows.""" answer = "" for key, value in self.ingredients.items(): answer += f"{key} ({value.get('abv')}%)\n" return answer def get_all_ingredients(self): answers = [f'{key} ({value.get("drink_info", "-")}%)' for key, value in self.ingredients.items()] return '\n'.join(answers)