Questionable Comments - podcast episode cover

Questionable Comments

Sep 18, 202342 min
--:--
--:--
Download Metacast podcast app
Listen to this episode in Metacast mobile app
Don't just listen to podcasts. Learn from them with transcripts, summaries, and chapters for every episode. Skim, search, and bookmark insights. Learn more

Episode description

Ben and Matt comment on different types of comments in code. Join our hosts and they explore both good and bad types of comments, from the essential to the inexcusable. Matt explains how to bump the failure counter to 99. Ben suggests violence against cats.

Transcript

Matt Godbolt

Hey, Ben.

Ben Rady

Hey, Matt.

Matt Godbolt

How are you doing?

Ben Rady

I'm doing good.

Matt Godbolt

Great to hear. Um, I've been looking at code recently, so, um, you

Ben Rady

You don't say!

Matt Godbolt

I mean, that's, I guess that's a, a significant part of my job these days as I've realized. I don't write as much code as I would like, and I look at a lot of code. And in particular as well, we have, uh, a number of, of folks who apply to work with us, and we ask them to do a nice little test. We, I, I think it's nice. It's nicer than the, some of the hacker ranking type stuff that's around out there that's, you know, reverses string in place kind of stuff. But anyway, it's a significant amount of code that folks write for us and, one of the things I'm always interested in is the why people are, uh, uh, write code in a particular way. There's, you know, engineering's a trade off. It's always a trade off, right.

It's not, uh, it's more of an art than a science, I think in a lot of cases, and deliberately, some of the things that I ask folks to write, uh, have many choices, and I don't necessarily tell them what the trade-offs are. And so it's always interesting when they then submit something to see and ask them why they made particular choices and sometimes those people will write giant comments above the code to explain the decisions that they made, because they're trying to express intent to me, the, the reader. And that's, that's great. And it just got me thinking, you know, I don't do that in my code when I'm writing code because, you know, presumably there's context around the people who are gonna be reading my code, have context, but it got me thinking how, what is a good comment?

Ben Rady

Mm-hmm.

Matt Godbolt

, what is a bad comment? What, what

Ben Rady

Is a bad comment? Right. Yeah. Why,

Matt Godbolt

Why do we write comments? Do we need them? Yeah. Are are they important? The compiler certainly, or the interpreter doesn't see them, so they can't be that important.

Ben Rady

Hopefully not.

Matt Godbolt

I mean, yeah,

Ben Rady

Yeah. Yeah. There is probably a whole category of like machine parseable comments,

Matt Godbolt

Doxygen things,

Ben Rady

And Yeah. You know? Yeah. Uh, doc test and, yeah.

Matt Godbolt

Oh, doc test is a great one. I, yeah.

Ben Rady

Yeah. And, and, and maybe that might be a little, uh, outside of the scope of what you're talking about here.

Matt Godbolt

No, I dunno, actually, I think it really, uh, it's another thing that's a good comment because it's a test and therefore

Ben Rady

Yeah.

Matt Godbolt

Is is expressing something that a computer can check for me, which is important, but that's not the only reason I write, uh, comments, obviously. So what's your thoughts on, but what makes a good comment?

Ben Rady

I mean, I, I'm in the same boat, I think as you is generally, I don't comment my code if I find myself, and, and I say generally 'cause I do, there are comments in my code for sure, but I, I don't make it a general practice to comment, you know, every method or every class or every, anything like that. And I, I generally find that comments represent a sort of deficit in the code that sometimes can be overcome by just writing better code and sometimes can't, uh, either because there literally isn't a way to do it, or I'm just not smart enough to do it.

Matt Godbolt

Um, right. The, the comment becomes an apology for the either complicated looking code that seems to be doing things that ought not to, or, or is, uh, is, is otherwise unintuitive. And so you have the comment explaining why look, before you do this, uh, before you edit this code, be aware that there's some spooky action at distance stuff going on here. And although it looks like you could replace it with a simple call to the standard libraries, blah, blah, blah, you can't because of X, that's a comment that is like an apology that says, Mike, I couldn't think of a clever way or a more intelligent way of doing this. But then the things that, the comments that I, sorry, I'm stealing from you immediately here, but like Yeah, you've, you've, you've, you've made me, made me think that yeah. The, the, the comment that says, um, apply interest rate, and then the next line is, uh, payment times equals 1.13.

That's not helping anybody. When I could say const auto, sorry, C++-sy interest rate equals 1.13. Uh, auto value, including interest equals, and, and, you know, oh, suddenly I've just given names to variables that are human readable and express the intention, and now the comment goes away. Which I think, you know, you and I agree on, on, on that. I think that's, that's one thing that one can do. And that's, I think what you're alluding to when you say, if you can think of a way of writing it in a way that exposes intent, then you can get away with a comment, very go away without a comment very often.

Ben Rady

Yeah. The best way to write code is in a way where both the computer and the human can understand it. Hmm. Right. Uh, instead of having to separate those two concerns. 'cause as we all know, in all things programming, creating unnecessary separation leads to things getting out of sync. And we know that no one has ever seen a comment that is out of sync with the code that it is immediately above. Right. So, uh, yeah, trying to unify that is a great way to make sure those things change together.

Matt Godbolt

Honestly, that's actually one of, together, one of the things that you mentioned that, and only the other day I caught myself reading, uh, a, a GitHub pull request, and the code had been updated and the, the comment was big enough that it was above the fold of where the diff came in, in the GitHub pull request. And I didn't realize that there was even a mention of like, something that was part of the code. And so of course I accepted it. It's like, this is fine. This is, looks completely reasonable to me. You've changed the code in that way. And then on a later viewing, when you're looking at the file in, in, in totality, you realize that the comment should have been updated as well. Then you just couldn't have been told from, from that. So there's a certain amount of context, uh, around it.

Ben Rady

Mm-hmm.

Matt Godbolt

The don't repeat yourself.

Ben Rady

The same, the information and comments. Yeah, exactly. Exactly. So, you know, ev it, it, it can sometimes feel a little pedantic, honestly. It's sort of like, okay, well I will name this function, uh, you know, calculate, uh, yearly loan interest instead of like, you know, calcint

Matt Godbolt

Exactly. Like,

Ben Rady

Maybe, maybe I should find a better way.

Matt Godbolt

Right. Right. And I mean, um, from a sort of C++ specific, or maybe not even C++ specific type of thing, but like another way that I find myself writing those sort of apology comments, is it in the explanation of like the interdependency between say, parameters to a function, like if you say, Hey, if you pass in X, you can't pass in Y as well. And so, like there's a thing that says precondition, either X must be truthy, or Y must be truthy, but not both. And you're like, well, and so you write that in the comment, and then maybe you even write like the assertion in the first line of the code that does that kind of thing. You're like, could I express my API so that it's actually impossible to, to, to pass both parameters.

Right. Could I just write two functions, one that takes an X, one that takes a Y and one that doesn't take either, and then maybe internally I'll implement them in the same function because of whatever reason, but like, the thing that I expose to the outside world doesn't even let you make that mistake anymore. So the fact that you had to kind of apologize and explain to somebody that this is not possible, goes away, uh, when they don't have the choice anymore. And you know, similarly with values and things, you know, uh, if, if you've got, like, this value must be, uh, greater than zero in C++, there are edge cases where you say if it's signed or unsigned and it doesn't work as well as you would like it to, but you can make types that are strong enough that to, to hold those, um, those, those, uh, preconditions in them.

You know, like, uh, for example, we have things like, uh, quantity classes, like a quantity of shares that can be traded and zero is not a valid quantity to be traded, neither is a negative number. Mm-hmm.

Ben Rady

Yeah. Yeah. Yeah. I mean, it's, it's, it's, some of that is again, like, you know, when I'm writing these kinds of things and I feel like there are sort of different categories of code apologies here, comment, apologies. And we could, we could dive into those. But when I'm writing those things, it is sometimes, I mean, it, obviously, it's very hard to tell the difference as a programmer between there's no way to do this and I don't know how to do this. Right. Right, right. Like, sometimes, you know, sometimes you have this sort of intuitive sense of like, I think there's a way to do this, but I just don't know what it is. But oftentimes it's just like, yeah, I, I'm, I can't, I don't know. Right. And so, uh, when you're sort of doing the, the one of like, okay, uh, I know there's a language feature that could categorically prevent this from being possible, and I am choosing for one reason or another to not use it.

Right. Either because of, you know, might be, would introduce a performance problem or because, uh, you know, you're just not familiar with the different edge cases and you feel like you might be, you know, pulling a regular expression and just trading one problem for another set of problems, or all the various reasons why you might choose not to do it. You might wanna put a comment in there saying, be like, I think there's a way to do this using blah, blah, blah, but I'm choosing not to do this because of X. Right. Um, but there's also just the, like, this is terrible and I hate it, but I don't know how to make this any better.

Matt Godbolt

That's a different, yeah. As you say, it's a different type of apology there,

Ben Rady

Right, Right. It's, it's just like, I'm sorry, I'm not a perfect programmer. This is all I have to give. Right.

Matt Godbolt

I, I can't quite remember myself, but yeah, we were on the train and we, uh, I was describing a story where I'd seen someone say, there was like a comment that says, it looks like you can do X or Y, but um, if you attempt to try and replace X with Y or do whatever, then once you've discovered it's impossible, come back here and bump the counter and it's the current value, you know, like 98, whatever,

Ben Rady

From 98 to 99 I

Matt Godbolt

I give up. Yeah.

Ben Rady

He, soever who can pull this function from the stone can be king, right?

Matt Godbolt

Yes. Oh, yes. That's, uh, that's it. Yeah. And I do wonder what that was now, but I mean, I've seen things like that before. Yeah. But one thing I would say about those apology comments, especially the ones that I have more of that second feel, um, and maybe this segues into some other reason where I put comments in the code, um, are to-dos like genuine to-dos. And you can have, I think we may have discussed some of these things before, but yeah, generally, you know, each project may have a different discipline around to-dos. I've worked on projects where to-dos are fine in branches, but you can't merge 'em into main. And that's like, you know, oh, we can't go to production while there's a to-do in the codebase. I've also worked on projects and the one that I'm currently on now, where to-do's a replace holder for this needs to be better, but in order to unlock some functionality that's really important.

And in order to sort of put a stake in the ground and say, this is some debt that I'm willfully accruing, and I know we've talked about debt and credit card debt or technical debt, various types of debt, but like, you know, colloquially, this could be improved. You put it to do in the code. And for me, the important part is to then make a, a, a GitHub issue to track it or add it to a general thing. And I put in a list and obviously that's that there you are repeating yourself, right? But there, I've got a number in the to do that references the, the GitHub issue. And then I try manually and certainly now I'm sort of on the project management side of things. I tag it as a to-do, and then there's a place to go to find all the debt and obviously git grep to do we'll find them all as well. And it's something that you kind of go, like, I'm putting my head up. It's not just me brushing it under the carpet

Ben Rady

Yeah. I, I think those kinds of comments, it's like there's a million ways to skin that cat. Uh,

Matt Godbolt

Don't skin cats.

Ben Rady

No, I keep saying using idioms, what are these idioms?

Matt Godbolt

You're an idiom!

Ben Rady

There's a million ways to solve that problem. Uh,

Matt Godbolt

I'm sorry.

Ben Rady

Uh, no, I love that. Uh, the, the, the, you can solve that problem in so many different ways. And, and the main thing for me is just having consensus among all of the people who are working on it. It's like, you know, whatever you pick todo means this, fixme means that. Right. We're gonna have links to issues. We're gonna have just issue number. Whatever you do, just do it consistently and make sure that everyone is doing the same thing. 'cause there's nothing worse than having, uh, code littered with these sort of like, well, one day we'll make this good type promises that, you know, are just hollow.

Matt Godbolt

That's it. That's the, they're unactionable. You can't go back to them.

Ben Rady

Right. There's no plan.

Matt Godbolt

And you know that you'll come back to it one day and the crash in production and you debug it down to the line that says, someday we should probably fix this thing. Yeah. I doubt this will ever happen in production though. So for now I'm leaving it and you're like, oh, oh dear

Ben Rady

Right, right, right. The thing that raises the error for the impossible case. Yes. And you're like, well, it turns out not impossible. Uh, yeah, totally. But yeah, all of those things, I feel like however you want to track those. 'cause the key thing there, I, I personally, I find a lot of value in drawing a strong differentiation between what I would call, call technical debt and just a lack of capability. I see those as two very different things. Technical debt is more of the sort of thing we were talking about before the former of like, I know there's a better way to do it, and I'm not, I'm just choosing not to do this. And, and it's not a question of like enhancing the capabilities of the system, but it's just like, I'm just writing bad code, and really you should just not be doing that period. But if you are, and you put a sort of an apology in there and explain, explain yourself, like, you know, there are maybe some situations in which it makes sense to do that very tactically, so long as you have the strongest discipline to come back and fix them quickly. Um, which is a whole other thing.

Matt Godbolt

Separate thing.

Ben Rady

Yeah. Right. Um, but the, the, the sort of like lack of capabilities of like, you know, um, we, there are certain types of input data that we could potentially receive here that we can't handle. Right. Right. And to

Matt Godbolt

Do Yeah. Handle cases, you know, packets of this type, you know, snapshot packets or whatever packets. Right, right. Okay. Yeah.

Ben Rady

And we just don't handle that right Now. You, I wouldn't think of that in most cases. I wouldn't think of that as debt so much as just like, that's just not a capability that you've built yet. Right. Now the debt might be like, okay, we don't handle these packets, and instead of raising a nice clear error that says like, we don't handle this type, and

Matt Godbolt

It just warns. Logger dot warn, and you're like, oh, why is my log file 200 gigabytes?

Ben Rady

Or it just proceeds through and it's like, yeah, exactly. Or it's like it ignores it or it drops it, or it just pretends like it's a different type. Like that is debt, that's a debt, you should not be doing that. Right.

Matt Godbolt

But the lack of capabilities is, but it's another good reason to put it, to do in the code and say like, Hey, the, I if only to reduce the scope of a pull request perhaps where you're sort of like saying like, Hey, I'm adding the functionality to even handle packets and I've, I handle heartbeat packets and I handle, you know, echo packets, but there's all of the other ones I don't do because that's a significant amount of work that may even be in a separate PR that I'm separately, God, I haven't wired them together yet, but to do all of the rest of these things And that can be part of the pr. Yeah, exactly. That makes it manageable. Yeah. So that's a different, that's a very different kind of comment from, from the, the explanatory sort of, uh

Ben Rady

Because those kinds of comments let you make smaller prs. And I think everyone kind of agrees that it's like when you, you know, the smaller your PRs can be, the more atomic your changes can be, the easier it is on your reviewers, the less risk there is to roll those things out. And so, like, whether you track those things in a GitHub issue that is linked in the code, or whether you just put it in a comment in the thing. Again, there's a, there's a million different ways to do that, but pick one and do it and be consistent about it. You know what I mean?

Matt Godbolt

Yeah. Yeah. So, so those are good comments. You know, they're, they're, but they, they sort of almost, uh, reflect a certain amount of project management or change management, uh, information in the code, right? But I think, so one of the questions we ask is what, what makes a bad comment? And to me, the, the obviously comments that are out of date or wrong are inherently bad, and they're worse than no comment at all. That, that's objectively, those are terrible comments. You know, something that says, uh, and now we sort the array and then you look down, you're like, it doesn't sort the array anywhere in this function whatsoever. I have no idea what it's talking about. Right, right. But then for me, the other thing that sort of, uh, winds me up in, and again, this is context dependent, because again, when folks are submitting code exogenously and they have to explain everything, that's the only mechanism that they've got, which is how we started this conversation.

I'm much more sympathetic to the explanation of the, of the, and everything about it. Here's the discursive nature of it. 'cause that's the only yardstick folks have got to, to let me know what they were thinking, right? Even though that's partly why if people, uh, get the, uh, if they write decent code and it, and it passes our test, then, you know, we'll probably have a conversation about it in person, and then they can tell me about it in person. Right. But that's, that's not the point. Um, but in, in a production code base where you are, you know, on a team of, of, of people, um, that, that have already have a agreed, a sort of level of understanding about what the systems all are, you don't have to continually explain, well, when I say this word, I mean this other system is in this, you know, I don't need that in every comment because you should know those things.

But the worst thing to see then is the, the essentially just a restatement of the code, but like the be beneath it, you know? Yeah. Adds to, you know, like, like our interest rate calculation from before. It's like, and now we do this thing, and then there's a block of code, and then, then we do this other thing and there's a block of code. And now I have written that for like more complicated things, but I think where the complication comes in, right? So I had a very performant, well, supposedly, very performant piece of code and I that I rewrote, it went from the obvious and straightforward way of doing everything to the, well, you wouldn't really want to do it any this way for real, but like, I know that I need to go incrementally in memory in order to, you know, stick to certain cache line things that I know and are important.

And this is not the obvious way of phrasing it, but again, this means that the branch prediction is done in a particular way and this is a constant time and then this thing Yeah. All those kinds of things. Right? And so there, I busted out the whole thing into, um, stages of a loop, and each part of the loop did have a small explanation of what each bit was going underneath it. And I felt that was more justified, but it's, it's on that knife edge. Um, if it wasn't so performance sensitive and it wasn't so, um, tangled because of the fact that I had willfully essentially in line what would've been many different functions into one loop to make it so that I could make these assumptions and pass things around by value and all that kind of stuff, then the obvious solution in general purpose code is if you have a little comment that says, this next block of code does X, Y, and Z is, you extract it into a function and you call the function that,

Ben Rady

Right, Yes.

Matt Godbolt

Which comes back to our kind of Yeah. And, and very often. So, um, I think it's, um, uh, Tony Van Arid has, uh, I think it was him, there was a c plus plus talk, and they, we were talking about like, how many times do you need to repeat a piece of code before you extract it into a function? And you know, people there with various hands going, you know, well the third time that you duplicate the piece of code, you should, you know, make a function in it. You know, how, when is it okay to copy that code around. Um, and um, his point was like, sometimes zero times of duplication, sometimes it's a piece of code that stands by itself. And even though it's not used by anything else in the entire program, it's useful to give it a name and define the things that it can and can't see what's in scope for it and the parameters that it needs to take that are transient, and then the things that are stateful that are inside the object, maybe it lives in or it's a free function or anything like that. And I'm like, that's a great way of also giving a name to a piece of code, you know, like part first we X, then we y then we Z becomes do X, do Y, do Z.

Ben Rady

Right. Right. Yeah. It's like, why do you put paragraphs and prose to make them easier to read? Right. It's the same, the same basic idea. Right. Right.

Matt Godbolt

It is, I mean, there's an argument there that says, you know, in the, in the prose sense, the paragraphs definitely make it easy to read, which is kind of what you're capturing when you start putting extra spacing and then you put a block of comments in front of each thing, you're kind of paragraphing it up then.

Ben Rady

Yeah. Except we have a better way to do that.

Matt Godbolt

We, yeah. And now the argument against that is that now you can't see the implementation of each of those line after line. You see a function call and another function call, then another function call. But if the name as an abstraction of what it's doing is accurate enough then you've, what you've got is the, the precis of what you're doing step by step. And of course, you can go any decent idea, well will let you go to the definition, or even some cases you mouse over the, the name of the function. Then it'll expand it out and show you, well, this is what that function is, by the way. And if it's only a few lines, then that's great. So I feel we have the, the tools and technology to, to, to do this.

Ben Rady

Yeah. And also every line of code that you wrote is also that, right. You don't

Matt Godbolt

I mean, and C++ you've overloaded every operator known to mankind, and you don't even know that one plus i is is doing what you think is doing,

Ben Rady

You know? Right. Right. Yeah, exactly. So like, you have to operate at some level of abstraction. You have to pick a level of abstraction. Right. Uh, you, you might as well pick a, a good one. That's, it's easy to understand and, and easy to read. Right. Right. Um, one of the other places, so you mentioned, you know, uh, you know, maybe blocking things like out like this out for performance reasons and kind of putting those things in there. Uh, and, and certainly I can imagine some scenarios, depending on what your compiler is doing for you, where, you know, extracting those things out into functions might have a material impact on the performance itself, or might make it harder to kind of pull some of the

Matt Godbolt

Right. I mean, let's just give compilers are really smart about doing this, but sometimes it's not feasible for them and you wanna give them a hand, but yeah,

Ben Rady

Yeah, yeah. Uh, another place where I do find myself writing these comments, which are, I, I don't even necessarily call them apologies. It's just sort of like, yeah this is just complicated is thread safety. Yes. So there, there are situations where you have, you know, multi-threaded, uh, code and, you know, the way that those threads might interact with each other is very non-obvious and a few choice comments to the reader to say like, Hey, make sure that you acquire this lock, and I've done my due diligence to try to design that into the API as much as I can, but sometimes you actually have to change the API and when you do make sure that you acquire this lock, because if you don't, then that's gonna create very, very subtle bugs, which will not show up in the unit tests. It might not even show up in any sort of system or smoke test until something catastrophic happens.

Matt Godbolt

That's a really good point. Um, and you, you make a very good point as well about the fact that, you know, sometimes that is, can be designed out of the API. Um, but, and, and I think, and certainly there are some annotations, I thinking like some Java annotations perhaps. Um, certainly there's, are

Ben Rady

You talking about like the synchronized keyword?

Matt Godbolt

No, no. I thought there were annotations that you could actually apply to something and say like the @protects and then some list of parameters, maybe just for static code analysis. Is that not something, or I've seen it,

Ben Rady

Maybe

Matt Godbolt

I've seen it in some language. Maybe it wasn't Java, but like I know it C++ in theory, there are a set of keywords or magical sacred comments that sort of allow static analysis to check these things, in which case that's even better, right. You can sort of encode it. Oh, yeah. I mean, there's still the why though. That's, that's, I think that ultimately all of the good comments that aren't procedural, like, you know, to-dos that we discussed all of the best kind of comments are the why. The thing that you really can't get the intent from this is like, it exists outside of the fabric of the code is the why on earth we would do it this way, or why on earth you need to do this, or why this is even something that is anyone would want to do.

Ben Rady

Right. Right.

Matt Godbolt

Um, not necessarily the how, because the how with the appropriate amount of scaffolding in the names of things. Um, and with, you know, just actually just reading the code, in theory, any human given enough time should be able to extract the, the, the, the how it's working and indeed what the, what it's doing more specifically, but, why, who knows, right?

Ben Rady

Yeah. Yeah. Yeah. Um, that's, that's a place that you should, you should, could comments can be super valuable in that case for for sure. Uh, one other kind of bad comment that I, uh, was gonna mention is when the comment is code, when you've just commented out code and then left it in the code and then committed it, and.

Matt Godbolt

Oh my gosh. Yeah.

Ben Rady

Why is this here? Like, we have source control for this. What did, why did you leave this here?

Matt Godbolt

Yeah. Yeah.

Ben Rady

Like, there's no reason to do that.

Matt Godbolt

I, I've seen that a lot with folks who aren't software engineers first and foremost or junior folks who haven't yet learned to trust their tools and they're, yeah. I keep toggling backwards and forwards between these two things and I'm like, okay. Um, first of all, if it's something that you don't, uh, you, you keep around just because there was that one time you needed it, then either you can stash it, get stash it, and then it doesn't need to be commented around, so it doesn't need to be checked in. Or if it's important enough that it does need to be checked in, make a command line flag to run it in that mode. If you can do it in that kind of thing. That's another thing I've, it's like, oh yeah, I just want to keep around, hang, hang onto to the intermediate files that this tool generates, you know? Right. It makes a temporary directory, writes a whole bunch of stuff in it, does some processing with it, and it outputs a single thing. And then it's really convenient to be able to look at the innerds sometimes when it's gone wrong or more, more likely when it's gone right. But you wanna see why it went right. And so it had deleted and you're like, well, stop commenting out the line that says delete the thing at the end. Make it a command line flag.

Ben Rady

Right.

Matt Godbolt

Um, or, you know, I've seen people hard code in, in the middle of their code, they've got the thing that refers to their, their like home directory and that they've commented out and said, yeah, it's really convenient to be able to like output the, the thing at this point here and like, ah, don't check it in.

Ben Rady

Yeah, no, please, please don't.

Matt Godbolt

But so, so yeah, I like commenting out large gobs of code because you're not sure about whether you might need it or not mean you could lean on your source control. And if you think you're gonna need it more often than not, then there's find a way to incorporate it so that it gets compiled along the rest of the code. It gets refactored automatically.

Ben Rady

Right. Exactly.

Matt Godbolt

Rather than just sitting in a comment and, you know, rotting away in a way that no one's gonna find. And maybe it's a useful function for somebody else too. Hey, if you thought it was useful to do this, maybe someone else will as well.

Ben Rady

. Yeah. There's a bunch of things like that. And, and I really, I do think that the, the most dysfunctional form of this is this sort of commented out utility main or method that people will selectively un comment and like, you know, put their home directory in or whatever. Like, not only does that create sort of, you know, accidental unnecessary noise when you like, you know, change the home directory and you, you, you commit the change just because it rode along with all of your other things, but also like, you know, there is a risk with these things. Like if you're using them for like real operational tasks that they get out of sync with the code in ways that won't fail cleanly. Right. Like, you would hope that if it got outta sync with the code and it was a compiled language, you know, you'd un comment it and you'd get some red squiggly lines or the compiler would give you an error. But especially in more dynamic languages, it's entirely possible to just run and do something that you absolutely did not expect it to do at all. Because the code has changed in the last six months and, you know, those functions you're calling don't work like that anymore. Yeah.

Matt Godbolt

That's an interesting one.

Ben Rady

So yeah, I, I, I think that's a, a really kind of like almost no excuse, right?

Matt Godbolt

I mean, you can make, you can put it in a branch that never gets to main, although, you know, again, still there it's gonna rot.

Ben Rady

Yeah. There's a million ways to do that. Right. But please, please, please do not just comment the code out and stick it in there and expect for it to live on forever. That's, not...

Matt Godbolt

So that actually reminds me of another comment type that is valuable. And it's a comment that you should never see ever in a checked in piece of code. And it is the comment that says, do not commit.

Ben Rady

Oh yeah. Uhhuh

Matt Godbolt

And it's really, really, really, so I was thinking again about the home directory type thing, like, despite all my protestations to the contrary, of course pragmatically up against the gun, sometimes I am gonna make some bespoke changes to the code just to exhibit a particular bug or to make something happen that just needs to be done right there and then, and while testing it or in the PR or whatever, um, you definitely don't want that to make it into, into production. So the, having a pre-commit hook and everyone should have a decent set of pre-commit hooks. There's no, there's some, there are plenty of choices out there for pre-commit hooks and managing them in a sensible way. My favorite is pre-commit, I think is the name of the thing. Pre-commit.

Ben Rady

Yeah, that's what I'm using too.

Matt Godbolt

It's a little python thing, um, that is very principled about how, uh, like little plugins for each of the things that you want it to do. Um, it's cool. Um, but there's husky, I think, in JavaScript land, and I'm sure there's millions of others, but anyway, have a pre-commit hook that says, Hey, don't let people check in stuff that says do not commit. And then it's fairly, fairly safe to, to then train people to say, if you are making the kind of horrific, you know, bool production equals true somewhere in the middle of your code base, right?

Ben Rady

Right, yeah. Right

Matt Godbolt

Immediately put a comment on the same line that says, do not commit. And then you know that you are being protected from the catastrophic check-in. That will will cause all sorts of issues. And of course, obviously it's incumbent on reviewers to make sure that these things don't make it through, but in the ebb and flow of things. I mean, we've all been there where you've reviewed a PR and said, yeah, this looks basically good to me. Can you just test this thing and make sure that that's, uh, comments aligned here or whatever. And then it's good. And then you've approved the pr and then of course the person's gone away and they've done what the, you know, some quick test tests before, you know, they've made a little change and they've tested it and they've thought, well, I better check that thing. And they, they make that kind of change and then they go, oh, everything's fine. They check it in and you're like, oh, I did not app. And unless you have the thing that makes you approve them every single time, you know, including the exact specific version, then, then there's a chance of, of sneaking it. Not even sneaking, I mean, accidentally getting something and everyone with the good faith was like, oh, sugar, I didn't mean to do that. But if you put do not commit, you're in a good way. Right?

Ben Rady

Yeah. Yeah. No, I love those. And honestly, the, the project that I'm working on right now, uh, we haven't added that and I kind of want it, we've gotten away with not having it. 'cause we do the thing, I think we talked about this in a prior episode, where, you know, we have todo comments and they, they mean very specific things and they can't be merged to main. Like there's a right check that prevents that, but there's nothing that would prevent someone from doing, like you say, the sort of production equals true. And you maybe like run that on your local machine for like a very bespoke focused experimental test and then you forget about it and then you check it in and now all of a sudden the thing that's running in the branch environment is accessing production data. Right Now, hopefully we've got our terraform set up to where it couldn't actually change any production data, but I don't wanna figure out if that actually works by doing that. Right.

Matt Godbolt

That's, yeah. There are some tests

Ben Rady

Now. Let's not test the seat belts by running into a brick wall. Right.

Matt Godbolt

Yes. Speak. Yeah. As we both have, uh, teenage new drivers. That's a bit top of mind for me.

Ben Rady

. Yeah. Right, right, right, right, right. Yeah,

Matt Godbolt

No, so, so yeah, that's, that's, I I can't think of any other type of comment that I've, oh, no, let's talk a little bit about, uh, documentation comments because I, I recently had a, a, a fairly, uh, interesting conversation with somebody on my team because they were putting, uh, comments, sort of doxygen style comments in front of stuff. And I'm like, I don't, no. And again, they, they tend to have a flavor, especially when there's things are when the, the, the linter rules. If you, if, if you're used to them say, oh, you need to document every parameter, but you know that, that very often you'll, you, you wanna document the function 'cause you wanna have like a one-liner that's human readable and that seems laudable still. Okay. Maybe, um, again, I would like to think that a name would hopefully carry that information, but this person was thought that was value in it.

And I'm like, okay, I, I'm, maybe I'll go with this. But then the sort of at param, uh, you know, Bob, the Bob to act on, you know, it's the function's called, you know, act on Bob and it's takes a param bob and you say, this is the Bob to act on, and it returns the acted upon Bob and you're like, uh, you know, that's not helping me. That's not useful. That's noise that's now six or seven lines above the function that are explaining to me obvious things. The point that they were making was that, you know, you mouse over and their IDE gives them a very nice explanation of everything and highlights and links all the types and things in a nice way when it's a doxygen thing. And so I'm, I'm warm to it. And in particular, the part of the codebase that they're working on has like a slightly more external, uh, visitor, sorry, external user kind of facing aspect to it.

And I'm like, well, okay, it's not the worst thing in the world to have some documentation around something. But, um, as you can hear in my voice, I'm still on the fence about it for all the reasons that it almost on purpose duplicates the code, but it duplicates it so that maybe someone who is not as intimately bound up with the, the code and everything to do with it can understand what that function is doing and can read a documentation that says, you know, this is how to, uh, I don't know frob the frobnicator.

Ben Rady

, this will come, uh, as not a shock to our listener in the slightest, but if you gave me the choice between getting docs with a library and getting the tests for that library, I'm gonna choose tests the 10 times outta 10. Yes. Uh, because I will find them much more useful and much more informative, and especially if I can edit them and rerun them, that will be extremely valuable. I do understand, and I can relate to the sort of, like, it's nice to have that sort of IDE popup that shows you that information. And I would say like, if you, you really wrote your doc strings or your whatever, whatever documentation system you're using to do this, if you really wrote that with the reader in mind, and you really wrote it in the context of this is what's gonna pop up when I hit, you know, control space or hover over this thing, and I want the reader in that specific context to have this information, I wouldn't be opposed to that at all. Right. In fact, I could maybe even see that as being useful. But I think unfortunately most of the tooling around this, you sort of like, you know, create the, start the start of the doc string and then your ID sort of fills out the template of the whole thing.

Matt Godbolt

That's right. Yeah.

Ben Rady

And then it's like you're, you either have to go and then change all of those settings and then share all those settings among everyone on your team. They're all using the same template for all of these things to make sure that it's not unnecessarily polluting your code with, you know, Bob parameter equals Bob. Um, or you have to do other things to sort of strip that out. And it just seems like a whole bunch of effort to take something that is marginally valuable and make it like slightly less annoying. Right. Right. Right. So, I don't know. I mean, it's, if it were, if it were me, I would just lean toward can we not? But I, if, if I had someone that was like, no, no, I really, I really want to commit to this, and yes, I've changed all the IDE settings and I've checked those, uh, you know, configs in and here's how you load them into your IDE and we're just gonna do this. I, I, I wouldn't, I wouldn't fight against that.

Matt Godbolt

Yeah. It sounds like we're of a similar, similar mind there. And I'll be honest with you in particular, doxygen, my heart always drops every time I click on the documentation for something, you know, an external, uh, thing, and I can open source project and it takes me to a document, a doxygen route. Uh, like the Boost, boost documentations, which is obviously one of the bigger libraries for C++ and a lot of the C++ standard features were first implemented in this Boost Library and then kind of adopted as part of the language. So things like shared pointers, so a, a lot of good heritage there and, but, um, it's a huge library and it's, it has a lot of complicated, um, functionality and the documentation is hit and miss. Some of them, you click on something and it goes to a big explanation page about like, Hey, you wanna use asynchronous io, this is a primer on what we mean by asynchronous.

This is how threads work. This is how our abstract. And you're like, this is great. I want all this stuff. Other times you just get to the, the head of, um, uh, a documentation tree that was generated entirely by reading the code and generating the Doxygen stuff. And it's all function name, uh, you know, from exactly as we described. Right. And it's just that on the other end, I'm like, well, I could guess that from the name of the thing I'm trying to work out how to use it. Can I have an example? Please? Can I see how things fit together? What, what is, you say that this is the bob to pass, but what is a bob? And you click on the Bob and it says, this is the Bob class. You're like, I know it's the Bob class. Thank you. I'm, I could read code, but I, and it, it's, it's frustrating and yeah. Documentation I guess is a whole other episode I would suggest.

Ben Rady

Yeah. And I mean, I, I would say, you know, one of the, the, you know, kind coming back to comments, like one of the good comments that you can have is if you have that type of documentation, either for a third party thing that you're using or maybe even something that you've built yourself that is maybe just hosted in a different place, having like a link to that type of documentation in the code sort of at the point where you would need it, is super useful. Right. And I, and I really appreciate, you know, most IDEs and editors and everything will just like open your browser when you like, you know, click on those things. Uh, and, and it's, and it's great to have, but, but yeah, the whole like, uh, one of my first jobs outta school was working at a company that sold graphics libraries, C++, Java, and uh, C sharp graphics libraries.

And part of what we would deliver to customers is like generated documentation. Right. Um, both like generated docs in, in the form of like, here's how you use this library. And we would do, you know, things with like the documentation system that we were using to create, you know, graphs and charts and diagrams and all these other things, but also just like the sort of, you know, method and class level documentation. And, uh, I never recall anyone ever complaining about that class level and method level documentation, which tells me that they almost certainly never read it. Yeah.

Matt Godbolt

That's funny. Yeah. Yeah. Documentation, documentation is difficult. And I mean, just to sort of finish up on that, that's one of the places where we started talking about like the, if you can put tests inside your comments above the thing, that's one place where I found that very valuable is the Python style doc test, where I can mm-hmm.

So let me ask you, what, what would you call a sequence of, uh, conversation pieces as we've just been discussing about comments? Would that be a, a collection of those? Would that be a commentary?

Ben Rady

Mm-hmm. Yes. Uhhuh

Matt Godbolt

Yeah.

Ben Rady

I have, I'm gonna have to think about this now.

Matt Godbolt

Well, dear listener, you will get

Ben Rady

So many pun activity, pun potential here. I can't pass up on this one.

Matt Godbolt

That's a good one. Uh, this is me throwing a gauntlet down in, in front of you there. So whatever this gets ends up being called. You know that Ben, Ben was responsible for it.

Ben Rady

I am the head of the Department of Puns here at Two's Complement.

Matt Godbolt

That is correct. That is correct.

Ben Rady

Yeah. Yeah. This is a good one. That was a good one. Cool.

Matt Godbolt

Well, uh, I'll catch up with you later in the week.

Ben Rady

Yep.

Matt Godbolt

Bye.

Transcript source: Provided by creator in RSS feed: download file
For the best experience, listen in Metacast app for iOS or Android