Gregory Shimansky wrote:
Geir Magnusson Jr. wrote:
Gregory Shimansky wrote:
In VM code SuspendChecker usage is clear enough, it is quite widely
used, although there is a lot of code which wasn't converted since
checker classes were introduced. New code uses them, old code uses
assertions.
It's not clear to a casual reader. I don't know if a rename is in
order, or the addition of a comment when used, but clearly to claim
that you added an "assert" and then use a class called "Checker" is
far from transparent.
geir
I'll try to make more descriptive commit comments :)
No - you're missing my point completely. it isn't about the commit
comments, but rather the source. Because of the unorthodox name of the
class and lack of any comment *in the code* it's unclear to a reader
what's going on.
It was certainly unclear to me.
The class is a cute trick, and I can see how it makes life easier for
maintenance, but we also have to consider that this code is being
written for collective ownership and maintenance, so the more explicit
and clear we can be, the better off the project is.
Always write code assuming that you win the lottery and retire tomorrow,
so make it clear enough that there are few questions.
geir
