I don’t know what I’m doing – a story about refactoring

I did a lot of refactoring the last week and after a comment on Twitter I decided perhaps it might be interesting for you to read about it.

Sometimes refactoring is straight forward like factoring out a method to make another large method more readable. Other times its a way with a lot of detours.

The problem

I’m currently working on a project to build mobile apps for a drone detection system. It periodically queries a REST endpoint of the detection system that’s running on a PC for the latest detection data, processes this data and shows the result on a map.

drone-ui

The app is structured according to RxVMS so we have a UI-, a Manager- and a ServiceLayer with Stream communication in between.

drone-architecture

The RtsaService does the REST call and JSON deserialization. The DetectionManager filters this data and creates graphical objects to display on the map on the UI, like Widgets,polygons and polylines.

The source file of the detection manager had about 400 lines and felt quite complex to overview. So I started thinking about breaking this manager into two, one that creates the UI objects and one that does the data processing. So I added a DisplayManager class and moved code over from the DetectionManager.
It turned out that the old DetectionManager only kept very little code while the new manager got most of the code. Not really what I wanted. Also I had to pass over a lot of data to the DisplayManager to enable it to access all needed data.

Not really satisfied I started to undo all this back to where I started but that still didn’t feel right. Haunted by ViewModels that I used in the past with Xamarin I just felt there should be another separation. So I added the DisplayManager again and moved the code over only to find myself in the same situation.

I pondered quit a while over this actually driven by nothing else but a gut feeling about better separation although there wasn’t a real need for it. In reality having it all in one class made a lot of things easier to achieve.

In the end I moved everything back into one class but extracted an abstract class as interface that contained all public methods making all others private. The result was astounding because it showed that the DetectionManager had only a very small public interface meaning it was easy to understand and to use. It would even be possible to easily mock it. Here’s how it looks now:

abstract class DetectionManager {

  // Emits an update on each processed sample that is cosumed by the MapView
  final mapDisplayUpdates = BehaviorSubject<MapDisplaySet>();

  // Emits an update on each processed sample that is cosumed by the TargetListView
  final targetListUpdates = BehaviorSubject<List<TargetsByCategory>>();

  // Should the target direction indicator be drawn with a bigger deviation
  bool get emphazyseDeviations;

  /// The average center of all detected [Antennas] we use that at startpup to move the map
  /// at that place.
  LatLng antennasCenter;

  /// Are all calculations relative to the current cuser's position
  bool calcBearingsRelativeToUserPosition;

  /// Initilization at startup has to be done before this Manager can be used
  void init({AppConfig appConfig});

  /// Start polling and processing of detection data
  void startSampleProcessing();

  void stoppPollingSamples();

  /// Switch the reference point for the calculations in aiming mode
  void enableAimingModeCalculation(bool enabled);

  void toggleObjectFocus(TargetDisplayData node);
}

The conclusion of this for me is not to change a design hastily just because it feels more right because of some software engineering paradigm but only to do it if there really is a need. Keep is simple!
If I had done it I even had violated the premise that I defined myself in RxVMS that managers do the business logic and provide data to the UI

I hope this might have been interesting for some of you.

(And yes I know the RxVMS series isn’t completed yet…)

Contact me:

Leave a Reply

Your email address will not be published. Required fields are marked *