search

bailuk / AAT

Another Activity Tracker for Android
https://bailu.ch/aat
GNU General Public License v3.0
150 stars 41 forks source link

Refactor Member Ignoring Method smell in Storage class #93

Closed emaiannone closed 4 years ago

emaiannone commented 4 years ago

Hi, I'm Emanuele Iannone, a master student at University of Salerno. Since my bachelor's thesis I have been working on a code smell refactoring plugin called aDoctor, which is able to identify and fix energy-related problems in Android apps. I launched it on your project, finding different instances of code smells. I chose one of them and let the plugin automatically fix it. In this case I chose Member Ignoring Method, that is present when a non static method does not use at all instance variables and other non static methods. These kind of smell may have a non trivial impact on energy consumption, as shown in this paper: https://www.sciencedirect.com/science/article/pii/S0950584918301678. Besides, this kind of refactoring does not impact on the functionalities of your app, so it is totally safe. Let me know if you are interested in this refactoring proposal.

MaxKellermann commented 4 years ago

Your idea to add static here is good, for clean code (and I like clean code), but I doubt it has any impact on energy consumption. Unfortunately, your link points to a commercial paper which I would have to buy. No, thanks. The abstract sounds like it's a Captain Obvious. I mean, duh, they really wrote a lengthy paper about something so trivial, and they want to sell it for money? Is your PR a funny attempt to improve sales of that paper? Or to improve sales of this plugin? About your code: you wanted to add static, but instead of doing just that, you additionally break the coding style of 3 code lines. That "smells" bad. If that's what your refactoring tool does, then no thanks. (Note: that's just my opinion, I'm just a contributor, I don't manage this project.)

MaxKellermann commented 4 years ago

Looking at some of your other PRs, it looks like your refactoring tool is indeed badly written. It always changes whitespace and coding style, several people complained about this already, and yet you still keep on submitting badly formatted PRs. Other PRs contain wrong and bogus changes which sometimes do not even compile. All you do is annoy project maintainers with trivial changes at best, but sometimes with build breakages, and coding style breakages all the time. This wastes other people's time. Removing static is good when appropriate (but not when methods may have an override in a subclass which your tool doesn't check), but explicitly inlining setter methods is harmful to the code quality. All of your (copy'n'pasted) PRs mention something about energy consumption and cite this Captain Obvious paper, but none of your code changes actually do anything to reduce the energy consumption of any app. Sorry, your work is not helpful for anybody.

emaiannone commented 4 years ago

I am really not interested in selling anything (apart from the fact that the paper can be easily found by just typing its name on Google) and I just wanted to validate the tool's capabilities and let some people know about it, without any bad intentions. I do know that it has some flaws because I am currently working on it: it's just a prototype. Anyway, I thank you for your feedback.

bailuk commented 4 years ago