A more structured approach to "how to validate stuff in source code comments"

[troessner]

I'd like to approach the "how to validate stuff in source code comments" more structured and analytical just for once ;)

Requirements for validation:

1.) Valid smell detector -> we already do that 2.) No garbage in the config hash which would in turn make Yaml.load crash. That's ready for review in #1101 3.) Validate config keys for a given, valid detector: 3.1) The basic smell detector options 3.2) The detector specific options 4.) Valid datatypes for a given key

So far, [1] and [2] haven been done in CodeComment, but going down that road further will become unmaintainable pretty fast. The problem is that the CodeComment is actually not only validating a comment but also SmellDetectors and configuration and what YAML loads and so on.

I suggest a separate class for doing this called CodeCommentValidator with the following API:

def initialize(comment:, line:, source:)
end

# Returns true / false and a message if comment is invalid
# @return [Boolean, String]
def valid?
end

Usage could then look like this:

validator = CodeCommentValidator.new(comment: comment, line: 5, source: 'string')
success, message = validator.valid?
raise BadCommentConfiguration, message unless success

And we could then catch only BadCommentConfiguration in Examiner as we do already.

WDYT?

[mvz]

I think CodeCommentValidator can just raise the error by itself, so we only need a #validate method (I just watched @chastell's talk and part of the 'burn your getters' talk he mentions, so I'm all tell-don't-ask at the moment). Apart from that nit, I'm all for this, since it means removing all kinds of extra knowledge and responsibility from CodeComment. :+1: :balloon: :heart:

[troessner]

I think CodeCommentValidator can just raise the error by itself, so we only need a #validate method

Good idea! I'll come up with a wip PR later and then we can discuss the API.

I think CodeCommentValidator can just raise the error by itself, so we only need a #validate method

Link pls?

[mvz]

Link pls?

Burn your getters: http://media.eurucamp.org/eurucamp/2013/jcoglan / https://github.com/jcoglan/burn-your-getters/tree/master

@chastell's talk: https://www.youtube.com/watch?v=pazYe7WRWRU

[troessner]

Awesome, thanks!

[troessner]

Quick update regarding:

1.) Valid smell detector -> we already do that 2.) No garbage in the config hash which would in turn make Yaml.load crash. That's ready for review in #1101 3.) Validate config keys for a given, valid detector: 3.1) The basic smell detector options 3.2) The detector specific options

[1] and [2] have been implemented, [3] is ready for review here

[troessner]

[3] has been done as well. I'm closing this issue to prevent it from becoming more messy and will create a follow-up issue for [4]