Code Review: Buffy The Vampire Slayer
Chapter 1 of Code Review
This marks the first article in an on-going series where I plan to examine and review code found in various forms of media. Film, TV, and stock photos love to throw code into any scene that calls for some kind of "technology"-vibe. But no one is ever expected to take the code too seriously. I plan to remedy this oversight by examining these instances of filler code, explain what the code is actually doing, and review it for relevance to the scene and context.
The Code
I'll be kicking off this series with a piece of code from the second episode of "Buffy The Vampire Slayer". In the episode, Harmony and Cordelia are in their school's computer lab struggling with a programming assignment. While Cordelia gossips about the "new girl" Buffy, we get a brief view of the following code on Harmony's computer screen:
Transcribing the contents of the screen gives us the following:
theRect->right = toPt.h;
}
else
}
theRect->left = toPt.h;
theRect->right = fromPt.h;
}
if (fromPt.v < toPt.v)
{
theRect->top = fromPt.v;
theRect->bottom = toPt.v;
}
else
{
theRect->top = toPt.v;
theRect->bottom = fromPt.v;
}
// The line hangs down and to the right
theRect->bottom += penHeight;
theRect->right += penWidth;
void CLine.CalcBoundingRgn(Point
The code consists primarily of two pairs of if/else statements that compare the v and h values for two objects fromPt and toPt. We can't see the entirety of the first if/else pair. But, in the complete if/else pair that we can see the code is checking whether the vertical value for a starting point (fromPt) is less than the vertical value of an end point (toPt).
In other words, the code is asking "Is our end point's height greater than our starting point's height?". If it is, we assign the end's vertical value to the top value in a rectangle object (theRect), and the origin's vertical value to the bottom value of the rectangle. If instead the source height is greater than or equal to the end height, we set the top of the rectangle to the end height, and the bottom to be the starting point height. In the obscured code, we can assume a similar check is performed for the left and right points of the rectangle.
After the if/else checks, we have a comment stating The line hangs down and to the right. And after that, we increase the lower-right values of TheRect by the penHeight and penWidth values. The final line of the code is incomplete, but it seems to be starting a new function definition CalcBoundingRgn.
So what is this code doing? It seems to mostly be a function for setting top, bottom, left, and right values for a rectangle object from two Point objects. At the end of the code, the rectangle's bottom right corner is increased by the dimensions of a pen, so I would assume this code is meant for a graphics program that draws a rectangle from two points on the screen.
Reviewing The Code
So is the code correct and free of errors? I'm not actually sure what language the code is in (it looks like some kind of C language), so it's difficult to get into specifics. From what we can see of the if/else blocks, there are no obvious errors and it all seems resonable and understandable. But there are still some parts that seem to be problematic regardless of language.
The code comment immediately below is incorrect for the context. It states that the "line hangs down and to the right". This must refer to the line formed by the to and from points, but at the point where this comment appears we can't assume the line is actually moving down and to the right. This is because the comment comes at the end of the series of if/else checks, and is at a spot in the code that runs regardless of the results of the previous comparisons. In fact, there's no one spot in the code where we can assume that the line is "down and to the right", because the check for whether the line runs "upward" or "downward" is completely independent of the check for whether the line runs "leftward" or "rightward". The code comment is incorrect because it suggests that the data is in a state that we can't actually guarantee from the logic shown.
Initially I assumed there was an issue with the offset performed for the penHeight and penWidth. My assumption was that because we perform an offset to the bottom-right corner of the rectangle's points, we would also want to perform a similar offset for the top-left corner and that this was missing from the code. However, this part actually makes sense because based on the operation performed, we can assume that the pen draws a rectangle moving up and to the left from it's given point based on its width and height. For example, if we wanted to make a mark at (30,50) on the screen with a pen width and height of 10, the rectangle drawn would have corners at (20,40), (20,50), (30,40), and (30,50). If this is how the pen draws, we don't need any offset for the top-left corner of our rectangle, because the pen drawing operation will perform the offset naturally. The offset calculated is simply done to ensure that the bottom-most and right-most points are offset in a manner symetrical to how the pen draws naturally.
What we are missing at this point in the code however, is any closing syntax. We start a new line with no indentation to define some new function (void CLine.CalcBoundingRgn()). But there's nothing closing the code above. The rest of the code is all indented inward, and we can assume it should be it's own function that needs a closing } brace.
Finally CalcBoundingRgn() is incomplete in a way that we wouldn't expect if they were near the end of their assignment (by the end of the scene, Cordelia celebrates completing the code). Harmony is still in the process of typing out the first line defining CalcBoundingRgn(), but only a minute or two later they've somehow completed the assigment. The name of the new function suggests it calculates a bounding range, and Harmony is only just starting the first line to define it. It's possible that this is the function we're seeing in the code above, in which case it should have already been written further up and would be a strange mistake to make. And if it's not the same function, it's hard to believe that Harmony was able to so quickly and easily type out the rest of the code for the assigment.
Relevance Of The Code
We don't know exactly what class the students are in, but it's some kind of computer skills or programming course. We can assume it's a required course because it's unlikely Harmony or Cordelia would be motivated to take a programming course if it weren't required. Cordelia at one point in the scene expresses frustration at having to take the course and asks "Why do we have to devise these programs, isn't this what nerds are for?" (Answer: Yes).
If this is a required course in their sophomore year, I'd assume it's an introductory level course. The general function of the code is reasonable for an entry level programming class. However, the specific content is too advanced for so early in the school year. It's explicitly stated in this episode that it's only the second day. The students shouldn't be working on anything much more advanced than a "Hello, World" program. Instead, they're apparently dealing with graphics output, pointers, functions, boolean statements, and more.
Conclusion
Overall, I'd give this code scene 3 out of 5 stars.
The code content is plausible for a high school level programming course, but the difficulty of the code doesn't align with what I'd expect on the second day of classes. The code itself is mostly correct and reasonable. It even shows clean and proper formatting which is something a lot of newbies to coding can struggle with (Good job, Harmony!). However, there are some syntax issues that would prevent the code from compiling and running. And even though the logic of the code is correct there is a comment that suggests Harmony is mistaken about the conclusions we can draw from the if/else checks.