"White" AST search: Add to Sub mutation #677
Conversation
| } | ||
| bool compilationDatabaseInfoAvailable = bitcodeCompilationDatabaseAvailable || | ||
| compilationDatabasePathAvailable || | ||
| bitcodeCompilationFlagsAvailable; |
There was a problem hiding this comment.
Could you explain the need for these three extra variables? It feels a bit too noisy and adds more mental overhead than needed.
There was a problem hiding this comment.
This is the only comment where I want to make an argument for actually doing this.
We can make the variables a little bit shorter but they actually make making of decisions below very clear:
We only enter the "Junk detection" decision only if compilationDatabaseInfoAvailable and we enter the "White search" decision if EnableASTSearch && compilationDatabaseInfoAvailable.
Otherwise I would have to introduce a variable called whiteSearchIsEnabled and put it where junkDetectionEnabled is set to true and we would have a duplication of both variables set to true to make their decisions.
This makes sense to me. For you?
There was a problem hiding this comment.
It does make sense to me, though there is a small misunderstanding on either side.
What I meant is to basically rename junkDetectionEnabled into compilationDatabaseInfoAvailable and drop the unnecessary variables. Then the same logic would be preserved, right?
I'm not going to die on this hill, so if having three extra variables works better for you, then I don't have any objections :)
No description provided.