Code review

Foreword

This is in part inspired by recent conversations I’ve had with several folks (1). While none of them were quite as… “interesting” as the fictional code review, they bear similar traits. Admittedly conversations like this can be rather frustrating — after spending time discussing ideas, it’s wholly unsatisfying to learn that all the prior discussions were based on some false premise.

But that’s life I guess.

As usual, a reminder that I am not a financial professional by training — I am a software engineer by training, and by trade. The following is based on my personal understanding, which is gained through self-study and working in finance for a few years.

If you find anything that you feel is incorrect, please feel free to leave a comment, and discuss your thoughts.

A different kind of code review

Subject: Code review of your Stack API.

John <john@pchouse.com>
to: Janky <janky@pchouse.com>

Hi Janky,

I am writing to you to do a code review about your new Stack API.

First of all, welcome to PCHouse! We’re happy you’ve joined us as our first software engineer to build out our technology backend! Now, even though I’m not a software engineer, I’ve worked at PCHouse for all my life, and since PCHouse works a lot with stacks, I feel I should share my experience here.

Here goes!

Line 10: Your comment calls this the “constructor” and sets “size” = 0. That is just weird. If the size is 0, you’re not really constructing anything, you know? I think you should reword the comment to read “plating”, since it starts out empty.

Line 15: Your comments call this the “destructor”. I think at this point, if there is anything left on the stack, it’s really just trash, so maybe call it the “cleanup”?

Line 25: I don’t mean to overstep my bounds here, but “push” seems like the wrong verb to call this method. I mean, I can sort of see where you are going, like you are “pushing on top of the stack”, but that’s not how it works in practice, you know? It’s really more like, you flip over the spatula and gravity kinda just takes over?

I think you were trying to avoid calling it too lengthy like “FlipOverSpatula”, but we can maybe go a bit less formal here to shorten the name. How about “Plop”?

As in, you flip over the spatula, and the contents just kind of plops down on top of the stack? I think this would make the API more self-documenting.

Line 30: As with “push”, I don’t really agree with “pop”. It sounds kind of violent, and I don’t think that’s what PCHouse is about, you know? We are more of a family-vibe kind of place. It’s a good thing there’s a friendly, family-oriented, PCHouse theme appropriate term here — “gobble”.

Just think about it:
The server plops on top of the stack, and the customer gobbles (from the top of the stack of course!).

It just makes it that much easier to understand, you know?


Best,
John


ps: What do you think about having a limit to how large size can be? It just seems dangerous to have a stack over 5 or 6 layers, you know? Gravity and all that.
Subject: re: Code review of your Stack API.

Janky <janky@pchouse.com>
to: john <john@pchouse.com>

Hi John,

Thanks for your warm welcome! I’m super excited to be here!

I’m a little confused. Why are we doing a code review over email? Do we not have something like Crucible, Collaborator, GitHub, etc.? If not, I’m happy to set us up. Let me know.

Also, I think you may be mistaken. “Constructor”, “Destructor”, “Push” and “Pop” are computer science terms, which describes the Stack. It’s actually clearer to use these terms in the context of the code.

Maybe we should meet face to face to discuss more?


janky
Subject: re: Code review of your Stack API.

John <john@pchouse.com>
to: Janky <janky@pchouse.com>

Hi Janky,

I think we’ll have to agree to disagree here. I understand this is code, but PCHouse is not a tech company — we are a family restaurant, our business is pancakes, and I hope you are not honestly suggesting you know more about stacks than me. 🙂

I think it may be instructive for you, to visit our kitchens to see the actual “plopping” going on, and maybe the dining area to see the “gobbling” as well. “Push” and “pop” just doesn’t have quite the same ring, you know?

In closing, most of our employees simply cannot relate to “push” and “pop”, but everybody understands “plop” and “gobble”. Think of it this way, you are writing code, but we are speaking English. So really, I think my way is best.


Best,
John

Understand each other

When you are chatting in a less formal setting, it’s generally fine to use whatever definitions you want for words. Stack::Plop() does have a nice ring to it, after all.

But sometimes that gets a little confusing, if the terms you use aren’t quite what you think they mean. It’s fine if you are just talking about finance (or any other topic) in broad, layman terms. But when you start trying to make use of known finance quantities, equations or rules, and fitting your definitions to those just by using the English meaning of the words, then you can get into weird situations.

There is a reason most technical domains (finance, computer science, etc.) develop, over time, a series of domain-specific terms that use English words, but have slightly (or sometimes, completely) different meanings from their dictionary meaning. It is often more precise and concise to discuss deeper issues using the technical terms, because they refer to well known and well understood ideas — it becomes easier and more efficient to build higher level abstractions and ideas using these terms, than simply English words.

I thought you thought I meant…

So, if you’ve spent the better part of an hour talking to someone about deep issues in a particular field, only to realize you don’t even agree on basic meanings of certain critical terms, then perhaps it’s time to step back and re-evaluate the discussion. If someone tells you “your understanding of the term X is not quite right — in this field, X means …”, then well, it seems to me that the only correct responses go along the lines of:

  • “I think you may be mistaken, I see from this <authoritative source> that they actually agree with my definition”, OR
  • “Oops! I hadn’t realize! Give me sometime to reconsider my position, and we can discuss again later”

Ok, technically, there is a third response, something along the lines of “I think we’ll have to agree to disagree, the English meaning of the term is what I meant”. But if you choose this third response, then know that trying to draw on established results in the field, based on that redefinition of X will not work. It simply makes no sense.

Another example

As a further example, if you define “ma” as “short form of ‘mother‘”, and you’d be damned before you change your mind, then I think even Newton will have a hard time trying to discuss his second law of motion. It just doesn’t work, you know?

Like, how would you do it? “F = ma” — mother is a forceful lady? mother is a force to reckon with? The Force is with mother (2)?

And it gets even more ridiculous if you try go a step deeper. Say, how are you going to define work done? “Work gets done when ‘ma‘ becomes ‘mad‘”?

Ok, on second thoughts maybe that one works (3). Darn it.

Footnotes

  1. There’s been several of these, over time. If you think I’m talking about you, you’re probably wrong.
  2. Happy May 4th!
  3. Not Physics advice.

1 comment

Leave a comment