1
\$\begingroup\$

I have been working on this code, which is the part of an economy system that handles the work command. This lets the user pick a role and if they have enough level then they will get it by updating a db.

I know it's inefficient and I am unsure how to reduce the if count and keeping a cyclomatic complexity below 15. I need to add 8 more jobs but I can't add anymore.

  • How can I reduce the if count?
  • What else have I done wrong?

The code I have been working on

  @job.command()
  async def apply(self,ctx,*,selection:str):
    a = banned(ctx.author.id,self.bot.blacklisted)
    if a is True:
      return await ctx.send("You are blacklisted from using this command.")
    db = client.bot
    posts = db.user
    for x in posts.find({"guildid":f"{ctx.guild.id}","id":f"{ctx.author.id}"},{ "_id": 0,"job":1,"level":1}):
      level = int(x["level"])
    if selection.lower() == "garbage collector" and level >= 0:
      posts.update_one({"id":f"{ctx.author.id}","guildid":f"{ctx.guild.id}"}, {"$set": { "job": "Garbage Collector","income":50}})#save
    elif selection.lower() == "cleaner" and level >= 7:
      posts.update_one({"id":f"{ctx.author.id}","guildid":f"{ctx.guild.id}"}, {"$set": { "job": "Cleaner","income":70}})#save
    elif selection.lower() == "car washer" and level >= 16:
      posts.update_one({"id":f"{ctx.author.id}","guildid":f"{ctx.guild.id}"}, {"$set": { "job": "Car Washer","income":105}})#save
    elif selection.lower() == "maid" and level >= 27:
      posts.update_one({"id":f"{ctx.author.id}","guildid":f"{ctx.guild.id}"}, {"$set": { "job": "Maid","income":145}})#save
    elif selection.lower() == "receptionist" and level >= 36:
      posts.update_one({"id":f"{ctx.author.id}","guildid":f"{ctx.guild.id}"}, {"$set": { "job": "Receptionist","income":185}})#save
    else:
      return await ctx.send(f"You are currently {level}, please go to school using ?learn")
    await ctx.send(f"You are now working as a {selection.lower()}")

The pymongo document format

"id":f"{message.author.id}"
"name":f"{message.author.name}"
"guildid":f"{message.guild.id}"
"cash":0
"job":"Homeless"
"balance":0
"shares":0
"level":0
"income":0
\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

Why loop?

for x in posts.find({"guildid":f"{ctx.guild.id}","id":f"{ctx.author.id}"},{ "_id": 0,"job":1,"level":1}):
  level = int(x["level"])

You're looping over the result of posts.find to... convert an element to an integer and then throw it away? Are you sure that this is what you want to be doing?

If you care about level, your current code is overwriting it on every iteration, so you'll only get the last one.

Don't repeat yourself

for job_name, level_min, income in (
   ('Garbage Collector',  0,  50),
   (          'Cleaner',  7,  70),
   (       'Car Washer', 16, 105),
   (             'Maid', 27, 145),
   (     'Receptionist', 36, 185)
):
    if selection.lower() == job_name.lower() and level >= level_min:
        data = {
           "id": f"{ctx.author.id}",
           "guildid": f"{ctx.guild.id}"
        }
        set_stuff = {
          "$set": {
              "job": job_name,
              "income": income
          }
        }
        posts.update_one(data, set_stuff)
        break
else:
    return await ctx.send(f"You are currently {level}, please go to school using ?learn")

If you want to get fancier, you can use a case-insensitive dictionary, but for this, that isn't strictly needed.

\$\endgroup\$
4
  • \$\begingroup\$ Thank you, is there a guide or something to learn how to make code like that? \$\endgroup\$
    – Axisnix
    Aug 30, 2019 at 6:27
  • \$\begingroup\$ Making what kind of code, in particular? Yes, there are some online Python guides. \$\endgroup\$
    – Reinderien
    Aug 30, 2019 at 13:52
  • \$\begingroup\$ Really efficient code like yours \$\endgroup\$
    – Axisnix
    Aug 30, 2019 at 15:02
  • \$\begingroup\$ It basically takes practice and experience. Read books and other peoples' source code, and keep submitting to CodeReview :) \$\endgroup\$
    – Reinderien
    Aug 30, 2019 at 15:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.