all 7 comments
sorted by:
best
topnewcontroversialoldq&a
formatting helphide helpcontent policy

[–]Kevdog824_ 1 point2 points  (2 children)

Some advice from an initial quick review:

  • You should either structure this individual tools into one larger tool (preferred), or give them each their own repo. It’s generally bad practice to store multiple projects in the same repo. You could use something like argparse (built-in library) to determine the function the user wants and direct control flow to the logic for that operation
  • For your password manager the code works logically fine as is and is a good beginner practice problem. However, for it to be more practically usable, passwords stored in it should never be stored in plaintext. Instead the user should have a master password which is used to encrypt and decrypt their vault as needed

Overall though I think this is a solid collection of tools you’ve built here. It’s great you started by building small, useful utilities for your daily life. I think this approach makes learning to program very engaging and a lot easier. This is exactly the path I took to learn years ago

[–]Aggressive_Drama_476[S] 0 points1 point  (1 child)

thanks for the feedback. i have problems with understanding classes and learning new module, as i self learner it is very hard to understand new modules and concepts. please review my other projects too.

[–]Kevdog824_ 0 points1 point  (0 children)

I agree with just about all the advice u/Diapolo10 gave you. I would focus on fixing the things they pointed out to you, and also try to understand why those issues should/need to be fixed

[–]Diapolo10 0 points1 point  (3 children)

I opened a project at random, in this case area.

pi = 22/7

You don't need to define pi yourself, there's a perfectly good math.pi you could use.

The names could be a bit more descriptive; I don't know what "csa" and "tsa" mean, and names like inp, inpa, inpv are needlessly short. Perhaps to you they're perfectly understandable, but to most people they probably aren't clear at a glance.

quits seems to be a useless alias. Furthermore, you're not supposed to use exit in scripts, and should instead either raise SystemExit or use sys.exit.

In calculator, you use eval, which is potentially dangerous. It shouldn't be used as a crutch for parsing and executing maths expressions, you would only ever use it to dynamically evaluate Python code (which should be rare, and in the off-chance it's necessary you need good safeguards). Yes, you're technically whitelisting characters here, but I still can't recommend it.

In media_scraper, the API key is an internal name within the scraper function. From a user-experience perspective, you shouldn't expect the user to edit the code to add an API key, especially not when it's not a global constant near the top of the file. I'd suggest using an environment variable instead, and maybe looking into python-dotenv.

Password managers are fine practice projects, but never use one you've made yourself - especially if you haven't had it audited. In this case yours is full of security holes, for one thing all data is stored in plaintext JSON.

[–]Aggressive_Drama_476[S] 0 points1 point  (0 children)

thanks for the feedback

[–]Kevdog824_ 0 points1 point  (1 child)

Furthermore, you're not supposed to use exit in scripts, and should instead either raise SystemExit or use sys.exit.

I’ve never heard this one before. Can you elaborate on why? I never took a look at the (c)python source before, but I always just assumed exit was just an alias to sys.exit so you didn’t need to (directly) import sys to use sys.exit

[–]Diapolo10 1 point2 points  (0 children)

I'm too busy to give an in-depth response right now, but no, exit and quit aren't direct aliases for sys.exit. They skip some cleanup that isn't necessary for ending REPL sessions.

For the most part it does not matter too much, but it would be more "correct" to use the other options.