Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding PMD plugin to tacoco #53

Open
tariq1890 opened this issue Nov 12, 2015 · 6 comments
Open

Adding PMD plugin to tacoco #53

tariq1890 opened this issue Nov 12, 2015 · 6 comments
Assignees

Comments

@tariq1890
Copy link
Contributor

Since you guys have added checkstyle, we could add the PMD plugin as well. I think PMD is a great tool for maintaining code quality and it keeps a constant check on the coding practices.

I ran the project with pmd and there were 31 PMD violations found. If Interested I can send a pull request containing the PMD Maven plugin and PMD violations fixed.
pmd.txt

@VijayKrishna
Copy link
Member

(looping @junghuk into the thread)
This is a decent idea. We will have to do this in stages -- specifically we should have individual commits for one violation at a time, especially if these violations cross cut the entire codebase.

Moreover, first I would like to incorporate check style to a reasonable level and then look to PMD. We might also want to have a discussion around any commonalities between PMD and Checkstyle in terms of the warnings they produce.

For now, i would hold off (stop) on the pull request, till we have completely explored both checkstyle and pmd. That said, lets keep this discussion alive.

@tariq1890
Copy link
Contributor Author

Sure Vijay! So in the other projects I have worked on, We used FindBugs, CheckStyle and PMD. All of them throw their own warnings and violations. If I remember well, there were hardly any warnings in common. It was pretty useful, as we had a comprehensive coding-standard enforcing system! Although its just one project where we used all the three.

@tariq1890
Copy link
Contributor Author

@VijayKrishna @junghuk Let me explain some more on the benefits I have seen using PMD. The most prominent advantage is that it encourages us to follow a proper object-oriented approach where we make sure there are clear separation of concerns and more focused modules with single purposes. It also ensures code readability. For eg:-

i)Consider the Cyclomatic Complexity rule of PMD which checks the number of If statements that may be used in a single class. Using too many If... else statements is a sign of weak modularity of code and a lack of well defined methods for the various operations we execute in our class.

ii)The Excessive parameter List rule indicates that a java object could be used to hold all the parameters which are otherwise declared in the method's declaration.

This way I feel PMD makes it very conducive to ensuring proper design of class structures and making code way more readable.

Also there are countless other rules in PMD which ensure we follow the best practices in Java.

Using PMD will really pay off in the long run when you need at more features to tacoco or even rework/redesign the existing components.

@VijayKrishna
Copy link
Member

@tariq1890 @junghuk: Based on the current state of the code base, which PMD rule(s) do you think is/are the most useful for this codebase? also confirm if such rules are already present in Checkstyle ... e.g. the Cyclomatic Complexity rule by PMD can also be done with Checkstyle: http://checkstyle.sourceforge.net/config_metrics.html#CyclomaticComplexity.
Similarly the Parameter rule is also available in Checkstyle: http://checkstyle.sourceforge.net/config_sizes.html#ParameterNumber

@VijayKrishna
Copy link
Member

I guess my point is that instead of looking to add more tools (like PMD, FindBugs etc.), i rather focus on the kind of rules that we want to impose on the codebase, and then the tool of choice becomes a secondary concern -- if that rule is available only in Tool X, we will use Tool X; or if that rule is available in the Tool that we are already using, then we do not have to add additional tools and be more focused on the rule itself.

Accordingly, the issues that we open should be more like #52 (Ensure Package Names ...).

@tariq1890
Copy link
Contributor Author

I do understand your point. As of now, I don't think there are any specific PMD violations that need attention (I do see quite a fewinstances of TooManyStaticImports occurring though). Its just that PMD has a lot of rules and checks and it would be beneficial when enhancements and features are added to tacoco in the future. It is also invaluable during refactoring work,

While there are overlapping rules between checkstyle and PMD, they are stil different tools with different puposes in mind.

Lot of interesting opinions in this SO page

http://stackoverflow.com/questions/184563/checkstyle-vs-pmd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants