11
votes
Accepted
Building Slack message
I've come back to this question about 4 or 5 times since it was originally posted, started writing up an answer, started doubting my alternative implementation, scrapped it, and then come back about a ...
- 8,049
6
votes
Building Slack message
We can learn a lot about what code is supposed to do by giving our functions explicit type signatures. Starting at the top, what are we working with?
team_id is ...
- 4,447
6
votes
Accepted
Summing categories of financial records per month in a query
Mixing Portuguese and English makes this code messy. Please pick one or the other. I recommend coding consistently in English, because Python, its library, and its code ecosystem are in English, so ...
- 146k
5
votes
Accepted
Cleanest way to get list of Django objects that have same foreign key and display them in a view
However, I am not sure if passing the interviews QuerySet like that is a good practice or not.
This is good practice, known as dependency injection. If someone asks for your business card, you wouldn'...
- 281
5
votes
Accepted
5
votes
Accepted
Let's make this Order nicer
There's a lot of duplication here for like items. A sign that loops and templates will
reduce redundancy.
There's a few distinct steps here. Better to separate each step to separate concerns. For ...
- 236
5
votes
Accepted
Get list of claims based on priorities
The function seems to be inconsistent; I don't know what is it intended to do, so if you say it works as expected, probably you can lose some functionality on unnecessary "beautification" of ...
- 2,404
5
votes
Alerts by phone calls for location-relevant Israel Home Front Command alerts
always running
The requirement is perfectly sensible,
but the implementation is weird and inconvenient.
make sure that the main program is running:
...
- 42.2k
4
votes
Django Rest Framework - ManyToMany relationship through intermediate model
Django-organizations is a great way to do this, if you didn't want to reinvent the wheel. Comes with many things I assume you would need in the future out of the box.
4
votes
Accepted
Tournament Management model
Here are few thoughts how the code could be slightly refactored.
The calculation of YEAR_CHOICES is little bit ugly. You could do it more elegant with a list ...
- 262
4
votes
Accepted
NHL Stats web app
At a high level you definitely have the right idea about pulling things out of your views. In MVC, we generally keep business logic out of views. That includes things like calculating player stats and ...
- 2,924
4
votes
Periodically syncing data from an API to database is extremely slow
Error handling
if response.status_code != 200:
print('api response error: {}'.format(response))
return
is better represented as a simple call to ...
- 71.1k
4
votes
How can I refactor the validation code to minimize it?
Exception handling
You may want to consider reducing the try block to where the expected exception actually is raised.
...
- 6,421
4
votes
Accepted
Email verification resend implementation Django
You have a problem in the post() method.
If the user does not exist you return self.form_valid(form), so if a new user in ...
4
votes
4
votes
Inventory Management System
This is a very naive approach to the issue and it's a race condition away from being a day+ debugging nightmare for you.
...
- 206
4
votes
Alerts by phone calls for location-relevant Israel Home Front Command alerts
Here are a few suggestions for function get_alerts:
Why is get_alerts an async Function?
I ...
- 3,626
4
votes
Alerts by phone calls for location-relevant Israel Home Front Command alerts
Context manager
One minor point: since you are opening files with a context manager (with:), file.close() is superfluous. The ...
- 8,313
3
votes
Web scraping news articles with Beautiful Soup
It seems wasteful to re-crawl the specified websites every time your web page gets hit, particularly if the web pages you're crawling have nothing to do with the request sent to your webpage. This ...
- 2,090
3
votes
Accepted
Django model to do accounting for a Litecoin miner pool
To expand on @200_success answer, you may want to have a look into Django's queryset annotation / aggregation and F objects.
So, basically, if you can work out the ORM equivalent to your ...
- 29.4k
3
votes
Django model to do accounting for a Litecoin miner pool
totalMachineHoursWorked should be a @classmethod. It could be written more succinctly like this, using ...
- 146k
3
votes
Cleaning service API using Django REST framework
Are the cleaners and clients supposed to be authenticated, maybe they should inherit from AUTH_USER_MODE or a base user? When using the ORM you want the object to have the name to the models on the ...
- 31
3
votes
Isolate a Database Change Within Django Transaction.Atomic
Let's start off with what's probably the most important note here: if you are running into issues with atomic transactions, pushing the database work off to Celery (or another worker thread) ...
3
votes
Accepted
Transactionally create something, if allowed
You don't need to use elif you can change that to just an if.
You can simplify code blocks by using guard clauses that have the following layout:
...
- 44.6k
3
votes
Django module to transform data from database to be displayed in Slickgrid tables
This first class shouldn't be important, don't think there is something here to improve.
You're wrong :)
There's a sea of repeated code here. You need to seriously DRY it up. I don't have enough of ...
- 71.1k
3
votes
Accepted
Counting users by the type in their profile
I'm creating a django
You probably mean that you're creating a web application on top of Django.
Given this code:
...
- 71.1k
3
votes
TDD tests for every view and its permissions in a REST API
As a side note, including your "includes" in the code you want reviewed makes it much easier to do so, this way we don't have to check if, for example APIClient and ...
- 12.7k
3
votes
Django model for real estate
As a rule of thumb in programming, "there are only four numbers": zero, one, two, and "many". You have seven photo fields, which means that you should just treat that as "many". Not only is that an ...
- 146k
3
votes
Optimising two for loops in Python
Assume you are referring to speed, not conciseness when asking about efficiency. In which case reducing the amount of loops will not necessarily improve performance.
...
- 151
3
votes
Accepted
A small protein database
This is really not bad at all. I only found a couple of things worth mentioning - run a linter which will tell you that you sometimes have too many blank lines inside of your function. Also, this:
<...
- 71.1k
Only top scored, non community-wiki answers of a minimum length are eligible