Feature Envy and Data Clump rules about oclint

[lqi]

Hi @joekain,
Thanks again for working on these two rules.

They are two very outstanding types of code smells, and I believe recognizing them is very important for improving code quality.

The biggest problem I could see is common C++/STL style tends to encourage some data clumping (especially with iterators) and feature envy of container classes.

I agree with you, and C++/STL gives us headache elsewhere as well. However, this is due to the lack of capability of filtering system headers or third-party libraries out of the analysis context. This is clearly something in this project's roadmap.

For these two rules, since they are so important, let's work together on them with baby steps. If you like, maybe you can create two pull requests for two rules, and we start with analyzing Objective-C only by keeping only VisitObjCMethodDecl in the AST traversal. Improve the test cases for Objective-C. And then start supporting C++ little by little. I am expecting to come up with the idea of dealing with std along the way.

Let me know if your thoughts, thanks.

[joekain]

Hi @lqi,

Starting with Objective-C only works for me. I'll try to find time this week to remove the C++ support and then send a pull request for Feature Envy.

Data Clump may take a little more time. I've found a problem with my implementation in that if it reports a clump of variables (a, b, c) then it will also report (a, b), (a, c), and (b, c) which adds a lot of unnecessary noise to the report. I'll put together a pull request with the current version if you like, but I don't think its ready to merge yet.

[lqi]

Starting with Objective-C only works for me. I'll try to find time this week to remove the C++ support and then send a pull request for Feature Envy.

Sure, thanks. Comment out C++ support for now, since our goal is to get back to this when Objective-C support is a little mature.

if it reports a clump of variables (a, b, c) then it will also report (a, b), (a, c), and (b, c)

Feel free to submit the pull request, you could start adding the test cases for these scenarios and let them fail, and then work to pass these tests.

Sincerely appreciate your time, @joekain.

[joekain]

if it reports a clump of variables (a, b, c) then it will also report (a, b), (a, c), and (b, c)

Feel free to submit the pull request, you could start adding the test cases for these scenarios and let them fail,
and then work to pass these tests.

Ah, great idea @lqi. I'll start with that.

[joekain]

Hi @lqi, sorry a month ago I started getting the FeatureEnvy pull request together but found I was unable to build the whole project. Then I ran out of free time. I was finally able to get back to this and thankfully was also able to build again.