search

tango-controls / JTango

TANGO kernel Java implementation. JTango moved to https://gitlab.com/tango-controls/JTango
http://tango-controls.org
8 stars 14 forks source link

fix AdminServerTest #36

Closed Ingvord closed 7 years ago

Ingvord commented 7 years ago

Resolve this TODO:

// TODO: manage tango://localhost:12354/1/1/1#dbase=no style device

This also fixes AdminServerTest

codecov[bot] commented 7 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@da4b9d8). Click here to learn what that means. The diff coverage is 78.78%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master     #36   +/-   ##
========================================
  Coverage          ?   10.4%           
========================================
  Files             ?     657           
  Lines             ?   51776           
  Branches          ?    7336           
========================================
  Hits              ?    5387           
  Misses            ?   45856           
  Partials          ?     533
Impacted Files Coverage Δ
...ava/org/tango/server/build/DeviceClassBuilder.java 78.26% <ø> (ø)
...ommon/src/main/java/org/tango/utils/TangoUtil.java 0% <0%> (ø)
...ain/java/org/tango/server/command/CommandImpl.java 67.5% <100%> (ø)
...java/org/tango/server/attribute/AttributeImpl.java 61.77% <64.28%> (ø)
...n/java/org/tango/server/testserver/JTangoTest.java 95.51% <76.47%> (ø)
.../main/java/org/tango/server/admin/AdminDevice.java 35.87% <80%> (ø)
...main/java/org/tango/server/servant/DeviceImpl.java 45.01% <81.25%> (ø)
...java/org/tango/server/admin/PollStatusCommand.java 87.03% <87.03%> (ø)

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 da4b9d8...98c54cf. Read the comment docs.

Ingvord commented 7 years ago

Ready for review!

@gwen-soleil , @bourtemb , @Pascal-Verdier , @JeanLucPons please give your input.

This PR is scheduled for merge in a week (10/08/17)

Ingvord commented 7 years ago

@gwen-soleil , @bourtemb , @Pascal-Verdier , @JeanLucPons

In this refactoring I would like to stress the common pattern you should probably use in every day programming:

First, you prepare all the data structures you deal with, and only then apply logic to it. This way code becomes much cleaner, easier to understand and maintain, because most of the functions become pure i.e. with no side effect + very well structured. Each function may be tested independently and it is much easier to simulate different inputs.

In this particular case we first obtain the device, please note the internal structure of tryFindDeviceByName method - it first aggregates all the devices and then applies filter to them. Here is a very nice book on the topic: link

Please note that this refactoring is not optimal, for instance gathering List of poll statuses must be instance method of DeviceImpl. I will fix it later.

Ingvord commented 7 years ago

Hi, I have created a gist from this PR: link