18
How is my Python code? (raw.githubusercontent.com)
submitted 21 hours ago* (last edited 21 hours ago) by the_citizen@lemmy.world to c/python@programming.dev

I don't know if it's the true place to ask, apologizing if not. I started to python one and half week ago. So I'm still beginner.

I made a terminal based weather application with python. What do you think about the code, is it good enough? I mean is it professional enough and how can I make the same functions with more less code?

Here's the main file (I also added it as url to post): https://raw.githubusercontent.com/TheCitizenOne/openweather/refs/heads/main/openweather.py
Here's the config.json file: https://raw.githubusercontent.com/TheCitizenOne/openweather/refs/heads/main/config.json

top 20 comments
sorted by: hot top controversial new old
[-] FizzyOrange@programming.dev 1 points 2 hours ago

Pretty good for a week and a half! I would recommend setting up a project using uv and also adding type hints and check them with Pyright (NOT Mypy).

[-] solrize@lemmy.world 9 points 18 hours ago

It's mostly ok, my immediate reaction is that it's unnecessary to put the whole program into a class. Python isn't Java. It's fine to write procedures at the top level.

Also as someone said, consider using enums, and also the new case statement as of Python 3.10. Type annotations also can make things clearer. I've been using mypy to check mine but there are some alternatives now that might be better.

There is not much error checking but that can be ok if you are the only user.

If the config file might be crafted maliciously, it can use escape codes for a terminal playback attack: https://www.cyberark.com/resources/threat-research-blog/dont-trust-this-title-abusing-terminal-emulators-with-ansi-escape-characters

Be careful of that.

[-] the_citizen@lemmy.world 1 points 9 hours ago

Yes I realized that I put everything to two classes. It's too much. I'll make more classes for more attributes.

I didn't know enums until now and it's awesome. I will do some exercises about it.

Last thing is interesting. I need to read that. Thank you for suggestions.

[-] solrize@lemmy.world 2 points 4 hours ago

There's no need to make more classes. You can move some of the code so it's not in any class.

[-] KindaABigDyl@programming.dev 11 points 21 hours ago

You appear to be afraid of spaces. Everything is cramped together.

Just bc you don't have to put a space after colons or after equals and commas and whatnot doesn't mean you shouldn't

Don't be afraid of spaces.

They make it easier for you to read your code when you come back later

[-] the_citizen@lemmy.world 1 points 9 hours ago

I revised the code. Added some spaces and comments for better readability. Hope it's better now.

[-] FizzyOrange@programming.dev 1 points 2 hours ago

Instead you should just use an autoformatter. Black is good. Ruff is probably good too but I don't have a lot of experience with it. YAPF is not good; don't use it.

[-] the_citizen@lemmy.world 3 points 21 hours ago

Oops, sorry. I will revise the code and place spaces. Thanks for suggestion <3

[-] taaz@biglemmowski.win 7 points 20 hours ago* (last edited 20 hours ago)

Usually, you would use a formatter anyway - it's good to know the standard way but for day to day coding I just have a shortcut bound that runs ruff format (you can even have it done automatically on file save).

[-] the_citizen@lemmy.world 1 points 9 hours ago

I need to search formatters. Thank you for suggestion.

[-] Derp@lemmy.ml 5 points 19 hours ago

Not the original commenter, but no need to apologise my friend. Nice work. Learning tip from me: give PEP 8 a read and save it for reference somewhere. It's the standard for how to format Python code, and future you will thank you for internalising it early on in your Python journey

[-] the_citizen@lemmy.world 1 points 9 hours ago

Thank you I will read it.

[-] cypherpunks@lemmy.ml 7 points 20 hours ago

I started to python one and half week ago. So I’m still beginner.

Nice work! Here are a few notes:

The WeatherApp object has a mix of attributes with long-term (eg self.LOCATIONS) and short-term (eg self.city) relevance. Instance attributes introduced in places other than __init__, which makes it non-trivial for a reader to quickly understand what the object contains. And, actually, self.{city,lat,lon} are all only used from the add_city method so they could/should be local variables instead of instance attributes (just remove the self. from them).

There seem to maybe be some bugs around when things are lowercase and when not; for example checking if self.city.lower() in self.LOCATIONS but then when writing there the non-lower self.ctiy is used as the key to self.LOCATIONS.

The code under if rep == "1" and elif rep == "2" is mostly duplicated, and there is no else branch to cover if rep is something other than 1 or 2.

It looks like the config only persists favorites so far (and not non-favorite cities which the user can add) which isn't obvious from the user interface.

Passing both location and locations into WeatherAPI so that it can look up locations[location] is unnecessary; it would be clearer to pass in the dict for the specific location. It would also be possible to avoid the need for LOWLOCATIONS by adding a non-lowercase name key to the per-location dictionaries that just have lat and lon right now, and then keeping LOCATIONS keyed by the lowercase names.

HTH! happy hacking :)

[-] the_citizen@lemmy.world 2 points 9 hours ago

That's very informative, I will rewrite the code with your suggestions. Thank you!

[-] Arkouda@lemmy.ca 2 points 20 hours ago

I am pretty new myself, but if you didn't know, here are some great free resources to help you.

https://inventwithpython.com/

[-] the_citizen@lemmy.world 2 points 9 hours ago

These are looking awesome. Thank you for sharing this <3

[-] Arkouda@lemmy.ca 1 points 4 hours ago

No problem at all! I hope it helps :)

[-] Endmaker@ani.social 1 points 20 hours ago* (last edited 20 hours ago)

Too lazy to check the logic but if possible, consider using enums.

[-] the_citizen@lemmy.world 2 points 9 hours ago

Yes I need to use enums. I realized it now.

this post was submitted on 25 Apr 2025
18 points (87.5% liked)

Python

7045 readers
50 users here now

Welcome to the Python community on the programming.dev Lemmy instance!

📅 Events

PastNovember 2023

October 2023

July 2023

August 2023

September 2023

🐍 Python project:
💓 Python Community:
✨ Python Ecosystem:
🌌 Fediverse
Communities
Projects
Feeds

founded 2 years ago
MODERATORS