Easer icon indicating copy to clipboard operation
Easer copied to clipboard

Refactor Member Ignoring Method smell in ConditionSerializer class

Open emaiannone opened this issue 5 years ago • 4 comments

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.

emaiannone avatar Jan 18 '20 11:01 emaiannone

Codecov Report

Merging #311 into master will increase coverage by 8.12%. The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #311      +/-   ##
============================================
+ Coverage      9.26%   17.38%   +8.12%     
============================================
  Files           430      443      +13     
  Lines         12373    12975     +602     
  Branches       1411     1517     +106     
============================================
+ Hits           1146     2256    +1110     
+ Misses        10985    10408     -577     
- Partials        242      311      +69
Impacted Files Coverage Δ Complexity Δ
...ge/backend/json/condition/ConditionSerializer.java 0% <ø> (ø) 0 <0> (ø) :arrow_down:
...ain/java/ryey/easer/core/log/ActivityLogService.kt 17.5% <0%> (-1.42%) 0% <0%> (-3%)
...ey/easer/core/data/storage/ProfileDataStorage.java 0% <0%> (ø) 0% <0%> (ø) :arrow_down:
.../easer/core/ui/data/script/EditScriptActivity.java 0% <0%> (ø) 0% <0%> (ø) :arrow_down:
...pp/src/main/java/ryey/easer/skills/SkillUtils.java 0% <0%> (ø) 0% <0%> (ø) :arrow_down:
...data/storage/backend/json/script/ScriptParser.java 0% <0%> (ø) 0% <0%> (ø) :arrow_down:
...aser/core/ui/data/profile/EditProfileActivity.java 0% <0%> (ø) 0% <0%> (ø) :arrow_down:
...ryey/easer/core/data/storage/EventDataStorage.java 0% <0%> (ø) 0% <0%> (ø) :arrow_down:
...yey/easer/core/data/storage/ScriptDataStorage.java 0% <0%> (ø) 0% <0%> (ø) :arrow_down:
...in/java/ryey/easer/core/ui/data/EditDataProto.java 0% <0%> (ø) 0% <0%> (ø) :arrow_down:
... and 217 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update eb61218...fa30b32. Read the comment docs.

codecov-io avatar Jan 18 '20 11:01 codecov-io

Thanks for the patch (though automatic I suppose, but I'll leave the comment anyway) :) Indeed, that method can be turned into a function, and it should be. Though, for this case, the impact should be minimum.

Anyway, I'm happy to merge it. But could you please recheck the intentindent? It seems to be incorrect.

renyuneyun avatar Jan 18 '20 14:01 renyuneyun

But could you please recheck the intent? It seems to be incorrect.

Do you mean a failing check? I think it is something wrong with codecov settings. It seems that coverage does't work since Multidex was enabled. I don't know how to fix it.

idealist1508 avatar Jan 18 '20 21:01 idealist1508