One of my favorite things about my company’s workflow is Pull Request reviews. Before anyone’s work is in, it is reviewed by at least one other engineer, and often more. And I don’t just mean “does it work?” reviewed, but actually, “is it good?” reviewed. This is a tale of a basic PR review, and some query optimization lessons while we’re at it.
The short version of what I was trying to accomplish: the backend for a new front-end feature that required annotating each instance of an object (a
PublishedCourse) in a
QuerySet with either “Registered” or “Complete” for a given user before passing it into the template.
PublishedCourse: bad. (I’m paraphrasing, obviously. My team is kinder than that).
published_course.course.id, the ORM lazily does a
SELECTon the Course. When you do
published_course.course_id, it doesn’t need to do this lookup, because it has already cached the
PublishedCourseobject. Main point: ORM eagerly downloads an object’s attributes (including related object’s IDs), but lazily downloads object’s related objects.”
I said this at the beginning, but let me reiterate — these code reviews are thorough. I got feedback at the level of not only general optimizations to reduce my number of database calls, but replacing
published_course.course_id because of the increase in efficiency it would provide. Now, that change in one place in the code might not make a noticeable difference today, but as I take these tiny lessons and am able to expand on them and apply them over the course of my career? What a difference. I don’t love making value judgements, but I feel pretty good about saying this: you should do PR reviews too.