Home - News ( United States | United Kingdom | Italy | Germany ) - Football scores

Showing content from https://github.com/pmd/pmd/issues/1962 below:

[core] Simplify Report API · Issue #1962 · pmd/pmd · GitHub

Summary of discussion with @pzygielo on #1960:

• Report::size/getNumViolations needs a comment because it's the size of the iterator
• Report::isEmpty doesn't necessarily mean that there are some violations, which is counter-intuitive (it considers violations and processing errors).

So maybe isEmpty is badly named. However it looks like the issue is a bit bigger. The isEmpty method is useless on its own, as it doesn't say anything about either the size of iterator() or of errors(), since the results are merged. It also doesn't take config errors into account which undermines its only usefulness (maybe checking that there is nothing to report, but with this implementation there can still be config errors). So you can't derive anything useful from a method call to isEmpty and will need another check (either hasErrors, or hasNext on the iterator, or a zero size() check).

So probably, isEmpty shouldn't even exist. But maybe the problem is even bigger.
The Report class appears to try being three lists at a time, as evidenced by the following methods:

• boolean isEmpty()
• boolean hasErrors()
• boolean hasConfigErrors()
• (notice no hasViolations)
• Iterator<RuleViolation> iterator()
• Iterator<ProcessingError> errors()
• Iterator<ConfigurationError> configErrors()
• int size()
• (notice no numErrors or numConfigErrors)

I shouldn't be mentioning the ReportTree and Metric methods, because they're supposed to be deprecated, but they aren't for now so let's just make a list to deprecate them later:

• treeIterator(), treeIsEmpty(), treeSize()
• metrics(), hasMetrics()

So Report attempts to be 5 collections at a time and does a meh job of it, because the API is assymmetric and very rudimentary.

I think making Report expose those collections independently would be a big upgrade to that API. We could replace all the above-mentioned methods by just

• List<RuleViolation> getViolations()
• List<ProcessingError> getProcessingErrors()
• List<ConfigurationError> getConfigErrors()

This would reduce greatly the API surface we have to maintain while improving the API available to users. Besides, since Report is implemented with Lists internally, it would be trivial to implement.

Making this change would supersede #1960

Should we do that?

RetroSearch is an open source project built by @garambo | Open a GitHub Issue

Search and Browse the WWW like it's 1997 | Search results from DuckDuckGo

HTML: 3.2 | Encoding: UTF-8 | Version: 0.7.4