Solution #880
Solution #880
Conversation
Le0n-K
commented
Nov 20, 2024
Le0n-K
commented
There was a problem hiding this comment.
Great job on implementing the task! 🎉 While there are a few minor issues, such as the typo in the template tag and the redundant increment of num_visits, these do not significantly impact the functionality. You've done well to meet the core requirements, and your attention to detail is evident. Keep up the good work and consider addressing the minor points for even cleaner code in the future! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| num_visits = request.session.get("num_visits", 0) | ||
| request.session["num_visits"] = num_visits + 1 |
There was a problem hiding this comment.
The num_visits variable is incremented twice: once when retrieving it from the session and again when updating the session. This results in an off-by-one error in the context dictionary. Consider incrementing num_visits only once and using the updated value both for setting the session and in the context.
|
|
||
| context = { | ||
| "num_drivers": num_drivers, | ||
| "num_cars": num_cars, | ||
| "num_manufacturers": num_manufacturers, | ||
| "num_visits": num_visits + 1, |
There was a problem hiding this comment.
The num_visits value in the context dictionary is incremented again, which is unnecessary since it was already incremented when updating the session. Use the num_visits variable directly without adding 1 again.
| <li><a href="{% url "taxi:manufacturer-list" %}">Manufacturers</a></li> | ||
| <li><a href="{% url "taxi:car-list" %}">Cars</a></li> | ||
| <li><a href="{% url "taxi:driver-list" %}">Drivers</a></li> | ||
| {% if user.is_authenticate %} |
There was a problem hiding this comment.
There is a typo in the template tag. It should be {% if user.is_authenticated %} instead of {% if user.is_authenticate %}. This will correctly check if the user is authenticated.
| {% for driver in car.drivers.all %} | ||
| <li>{{ driver.first_name }} {{ driver.last_name }}</li> | ||
| {% endfor %} | ||
| {% for driver in car.drivers.all %} |
There was a problem hiding this comment.
The iteration over car.drivers.all should use parentheses: car.drivers.all(). This is necessary to call the method and retrieve the list of drivers.
