Checking for a condition, even if it's always true

Question

In my program there is the following piece of code where I must delete a ' ' from the second to last index in a StringBuilder:

builder.deleteCharAt(builder.length() - 2);

I do not surround it with checks to either condition builder.length() >= 2 or builder.charAt(builder.length() - 2) == ' '), because I am 100% sure that both these conditions will always be true, even though at first glance, it is not so obvious.

I'm torn between feeling that just because I have a higher understanding of the code, doesn't mean I should neglect the conditions for the sake of others in the team. But at the same time, if I do check for these conditions, it would be absolutely unnecessary.

My solution is to simply add a comment, but I was wondering if is this considered to be good practice?

builder.deleteCharAt(builder.length() - 2); // should always be ' '

• mdfst13: Why should it always be a space? That's the thing that you could comment that would be helpful. Also, why does it matter if it's a space? A bald statement that it should be won't be helpful if the code changes and it stops being a space. We might be able to review this better with more context. Note that moving the section of code that guarantees that that character is a space to its own method would allow you to write a unit test against that. Then changes in the code would cause the unit test to fail. Put the comment on the unit test to explain why you wrote it.
• budi: It's kind of hard to explain the problem without a lot of context, but I'm essentially creating a text formatter. The main thing here is that if the character at that position is not a space, it is going to either a letter/number or punctuation (which I don't want to delete!). I believe assert is a great way to handle this problem as stated below.
• h.j.k.: Is there an XY problem here? Are you concatenating the delimiter ` ` in a loop, while needing to remove a redundant one? If so, I suggest asking a new question about that loop, and maybe there's a better solution for eliminating this 'workaround' altogether.
• budi: I'm don't believe it's an XY problem. So here's a shortish example from my project where I try to correct an acronym. I've split the current line by both space and punctuation, for example the String "A.M." becomes ["A", ".", "M", "."]. After processing the first three tokens, the StringBuilder holds the string A . M and the current token is ".". Based on certain flags I've set, I determine I've come across an acronym. So here is where I call builder.deleteCharAt(builder.length() - 2); to delete the second to last character in the StringBuilder.
• mdfst13: You shouldn't be afraid to post context here. We aren't like Stack Overflow -- you don't need to reduce to a minimal example. We mostly review entire programs. The problem with this question as stands is that it has too little context. You haven't even given us the entire method.

Answer 1

Don't use a comment. Use code. Specifically, assert builder.length() >= 2 and assert builder.charAt(builder.length() -2) == ' '. Assertions will clearly indicate the preconditions, and they can be excluded by the compiler or jvm so they don't impact the deployed code.

Answer 2

I'm torn between feeling that just because I have a higher understanding of the code, doesn't mean I should neglect the conditions for the sake of others in the team.

That feeling you have is "encapsulation" or the principle of least information, or whatever kids call it these days.

If this problem is occurring across files, modules, classes, functions, or some other boundary you feel establishes a "library/client" relationship, then the StringBuilder-manipulating code should not assume your precondition, and you definitely should not assume clients of your code are going to open up your implementation and read your comment to learn what they can't do.

Better options include:

• document the precondition in the function's documentation.
• use an assert.
• throw an error (in Java this would mean don't do any check).

If the problem is occurring within one function or file, then either way is fine. For instance in this block:

if(!myString.isEmpty()) {
 // do something else...
 System.out.println(myString[0]);
}

You will notice that the reader may very well have forgotten about the "if"-check. That's okay. There's a line somewhere where the reader needs to have some knowledge of the code. That being said:

• your "should always be " "" comment is not very good. It doesn't tell me why, and of course comments do not run so I do not even know if I trust it.
• Code is better than comments. I think an assert is just as bad because it leaves me with the question "why won't this assert fire?" which is the same question I had when read your code anyway.