Showing posts with label complexity. Show all posts
Showing posts with label complexity. Show all posts

Sunday, 27 October 2019

What's in a name?

You've probably seen the old joke.
There are only 2 hard problems in Computer Science:

  1. Naming things
  2. Cache invalidation
  3. Off-by-one errors

But like every good joke there's a substantial core of truth in it. And there's a good reason why Naming Things is top of that list. It's hard. And with most geeks being complexity-addicts, we like to ladle additional difficulties on top of the naming problem; for example:

  • Compiler restrictions - Java insists on one-class-per-file, with a match between filename and class name
  • Linter rules - Requiring interfaces to start with I or implementations with Impl
  • Usability problems - if all your files are called index.js (because they live in different directories), navigating your text editor's tabs becomes a challenge
  • Readability problems - Spending too long with AbstractInjectedDAOFactoryDecorator and AbstractInjectedDAOFactoryDecoratorBuilder-type names will wear out your sanity even faster than your keyboard

But the thing is, it's worth it. Especially in the increasingly-decomposed, microservicey, lambda-esque landscape, being able to latch onto a variable and/or class because its name is well-chosen and descriptive, can be the difference between a laser-like open file by name, make change, test, commit, DONE and a flailing find in all files, open a dozen of them, trace execution path, make speculative change, get it wrong, repeat until somehow fixed loop.

Better yet, as your team grows in experience, your shared vocabulary grows with it. Those well-named classes become a shorthand; you can use them as examples or references and everyone knows what you're on about - much like the Gang of Four Design Patterns are/were all about. I say "were" because of the 23 original GoF patterns, I would argue only a handful are in common usage today and have achieved the universality of understanding the authors were hoping for: Singleton, Iterator, Observer (although probably referred to as publisher-subscriber), Adapter/Proxy (often used interchangeably) and Factory Method.

I note that the design patterns that have truly "taken off" are without exception, the "simple" ones. I mean, look at Adapter (diagrams from the Black Wasp site also linked above):


... and now compare with Abstract Factory:

All those classes. All of them need names. And in Java at least, each one living in its own file. Possibly in different directories too. Is it any wonder that people have shied away from these complex patterns?

Saturday, 31 March 2018

Green Millhouse - Fixing the OpenHAB BroadLink Binding (part 1)

You can follow along at Github, but my rebuilding of the Broadlink OpenHAB binding is nearing completion.

I've been building and testing locally with my A1 Air Quality Sensor, and since fixing some shared-state issues in the network layer, haven't yet experienced any of the reliability problems that plagued the original binding.

For reasons that aren't clear (because I'm working from a decompiled JAR file), the original binding was set up like this in the base Thing handler (which all Broadlink Things inherit from):
public class BroadlinkBaseThingHandler extends BaseThingHandler {
   private static DatagramSocket socket = null;
   static boolean commandRunning = false;
   ...
}

As soon as I saw those static members, alarms started ringing in my head - especially when combined with an inheritance model, you've got a definite "fragile base class" problem at compile-time, and untold misery at runtime when multiple subclasses start accessing the socket like it's their exclusive property!

An attempt to mitigate the race-conditions which must have abounded, the commandRunning boolean only complicated matters:
    public boolean sendDatagram(byte message[])
    {
        try
        {
            if(socket == null || socket.isClosed())
            {
                socket = new DatagramSocket();
                socket.setBroadcast(true);
            }
            InetAddress host = InetAddress.getByName(thingConfig.getIpAddress());
            int port = thingConfig.getPort();
            DatagramPacket sendPacket = new DatagramPacket(message, message.length, new InetSocketAddress(host, port));
            commandRunning = true;
            socket.send(sendPacket);
        }
        catch(IOException e)
        {
            logger.error("IO error for device '{}' during UDP command sending: {}", getThing().getUID(), e.getMessage());
            commandRunning = false;
            return false;
        }
        return true;
    }

    public byte[] receiveDatagram()
    {
        try {
            socket.setReuseAddress(true);
            socket.setSoTimeout(5000);
        } catch (SocketException se) {
            commandRunning = false;
            socket.close();
            return null;
        }

        if(!commandRunning) {
            logger.error("No command running - device '{}' should not be receiving at this time!", getThing().getUID());
            return null;
        }

        try
        {
            if(socket != null)
            {
                byte response[] = new byte[1024];
                DatagramPacket receivePacket = new DatagramPacket(response, response.length);
                socket.receive(receivePacket);
                response = receivePacket.getData();
                commandRunning = false;
                socket.close();
                return response;
            }
        }
        catch (SocketTimeoutException ste) {
            if(logger.isDebugEnabled()) {
                logger.debug("No further response received for device '{}'", getThing().getUID());
            }
        }

        catch(Exception e)
        {
            logger.error("IO Exception: '{}", e.getMessage());
        }

        commandRunning = false;
        return null;
    }

So we got a pseudo-semaphore that is trying to detect getting into a bad state (because of shared-state), but itself is shared-state, thereby experiencing the same unreliability.

Here's what the new code looks like:
public class BroadlinkBaseThingHandler extends BaseThingHandler {
    private DatagramSocket socket = null;
    ...

    public boolean sendDatagram(byte message[], String purpose) {
        try {
            logTrace("Sending " + purpose);
            if (socket == null || socket.isClosed()) {
                socket = new DatagramSocket();
                socket.setBroadcast(true);
                socket.setReuseAddress(true);
                socket.setSoTimeout(5000);
            }
            InetAddress host = InetAddress.getByName(thingConfig.getIpAddress());
            int port = thingConfig.getPort();
            DatagramPacket sendPacket = new DatagramPacket(message, message.length, new InetSocketAddress(host, port));
            socket.send(sendPacket);
        } catch (IOException e) {
            logger.error("IO error for device '{}' during UDP command sending: {}", getThing().getUID(), e.getMessage());
            return false;
        }
        logTrace("Sending " + purpose + " complete");
        return true;
    }

    public byte[] receiveDatagram(String purpose) {
        logTrace("Receiving " + purpose);

        try {
            if (socket == null) {
                logError("receiveDatagram " + purpose + " for socket was unexpectedly null");
            } else {
                byte response[] = new byte[1024];
                DatagramPacket receivePacket = new DatagramPacket(response, response.length);
                socket.receive(receivePacket);
                response = receivePacket.getData();
//                socket.close();
                logTrace("Receiving " + purpose + " complete (OK)");
                return response;
            }
        } catch (SocketTimeoutException ste) {
            logDebug("No further " + purpose + " response received for device");
        } catch (Exception e) {
            logger.error("While {} - IO Exception: '{}'", purpose, e.getMessage());
        }

        return null;
    }    


A lot less controversial I'd say. The key changes:
  • Each subclass instance (i.e. Thing) gets its own socket
  • No need to track commandRunning - an instance owns its socket outright
  • The socket gets configured just once, instead of being reconfigured between Tx- and Rx-time
  • Improved diagnostic logging that always outputs the ThingID, and the purpose of the call


The next phase is now stress-testing the binding with a second heterogeneous device (sadly I don't have another A1, that would be great for further tests), my RM3 Mini IR-blaster. I'll be trying adding and removing the devices at various times, seeing if I can trip the binding up. The final step will be making sure the Thing discovery process (which is the main reason to upgrade to OpenHAB 2, and is brilliant) is as good as it can be. After that, I'll be tidying up the code to meet the OpenHAB guidelines and hopefully getting this thing into the official release!

Friday, 29 September 2017

Stack Evolution part 1

It wouldn't take too close a reading of this blog to determine that I'm a huge fan of the Play Framework. For the past 4-or-so years it has played a pivotal role in all of my Millhouse Group projects, and frequently during my "day job" as well.

The "revenue-earning" configuration I've rolled out time and time again looks almost-unfailingly like this:

Javascript JQuery, Moment.js, etc
"Presentation" JSON/AJAX HTML/LESS
Controllers Play (Reactive, Async)
Services Play - RESTful API calls Mondrian
Persistence MongoDB (via ReactiveMongo)

Yep. Decidedly unsexy, and yes, occasionally the javascript can get a bit funky and coupled to the HTML, but it works and with careful attention to the principles of Single Responsibility and Least Surprise, a "vertical slice" through the functionality would look like:

Javascript /assets/js/author.js
"Presentation" /views/html/author/author.scala.html
/assets/css/author.less
Controllers /controllers/AuthorController.scala
Services /models/Author.scala
/services/AuthorService.scala
Persistence [Authors collection in MongoDB]

... where no source file is more than 200 lines of code. Nothing too controversial there, I think you'd agree.
However...
My exposure over the last 18 months to React.js has truly opened my eyes to the potential of a true front-end application (as opposed to the very 2010-era progressively-enhanced-markup approach I've described above). In the next post I'll show the architecture I've been calling the CRAP-stack which has been making working on the Javascript front-end as pleasant as doing the heavy-lifting with Scala in the back-end (i.e. very!)

Thursday, 29 September 2016

Push notifications and the endless quest for "rightness"

I recently was tasked with getting a push notification system up and running (e.g. the "unseen" badge count and messaging you see in lots of mobile apps). We had a fairly simple polling-based notification system that worked quite well but we really wanted that next level of connectedness that you get from instantaneous notifications, even when the app is "closed".

My attempt to get this working basically extended the existing system which used a count and some Booleans. After countless hours of trying to get this sync right, I came to a realization:

Trying to keep state by passing Booleans back and forth is like flinging paint at a wall and expecting the Mona Lisa to appear.

I had seenLatest, I had hasChanges and even resorted to forceUpdate. There was always a corner-case or timing/sequencing condition that would trip it up. And then I realized that I needed to embrace timing. Each syncable thing has a lastUpdated time and each client has a lastSaw time.

Key for getting this working was remembering to think about this from a user's point of view - they may well be logged in on three devices (aka clients) but once they have seen a notification on any device they don't want to see it again.

The pseudocode for this came down to:

Polling Loop / When a thing changes

  userLastSawTimestamp = max(clientLastSawTimestamps)
  
  if (thing.lastUpdated > userLastSawTimestamp) {
    showNotification(thing)
  }

On User Viewing Thing (i.e. clearing the badge)

  clientLastSawTimestamp[clientId] = now


And now it works how it should.

And I see failed attempts to do it correctly everywhere I look ... :-)

Tuesday, 28 April 2015

Strongly-Typed Time. Part 1: Rationale

I've quite recently become involved in an after-hours project that has a strong temporal component to it. Basically every interaction with the system will need to be labelled with a time, and they will constantly need to be compared and converted. Add to this the fact that the first beta customers are located on opposite sides of the Pacific, and that events can occur in a further 3 European countries, and a way to safely and unambiguously represent the time of something happening in a time and a place seems paramount.

While Joda Time has undoubtedly made date/calendar/timezone manipulation a happier task for the JVM developer, I'm looking for something stronger. I can pass around a DateTime all day long (no pun intended) but until I inspect its TimeZone I can't be sure where it originated from, or if it is in fact the canonical UTC.

As a result, there is nothing at compile time to stop me doing something like:
  def displayAllEventsBefore(allEvents:Seq[Event], threshold:DateTime) = {
 
    // allEvents have been normalized to UTC. But there's no way of knowing this:
    allEvents.filter(_.isBefore(threshold)).display    
  } 

  // ... much later, miles away
  val myTime = new DateTime() // Happens to be in "Europe/Paris"

  displayAllEventsBefore(events, myTime)

Which will work just fine most of the time, except when there's been an event in the last hour, when we won't see it. Or is it the other way around? Tricky, isn't it?

There's nothing in the type system to prevent these kinds of runtime problems. It comes down to developer diligence in naming/commenting/testing all the things - literally, every thing that uses a representation of time - to ensure correctness.

But hang on, aren't compilers really, really good at ensuring correctness?

Wednesday, 10 September 2014

Why Clojure is fascinating me

So I've hopped aboard the Clojure boat, as it's the preferred implementation language for "new stuff" at work.

And I'm liking it. A lot. Possibly because of the way we're using it (microservices), but probably just intrinsically, it is a language that seems to fit in the head very nicely. Not encumbered by special cases, exceptions, implicit magic and overloads. (Don't worry, I still enjoy Scala, but it's a very different Kettle[Either[Throwable, Map[String, Fish]]]).

The succinctness and elegance of Clojure is also thrown into sharp relief by the other thing I seem to be spending a lot of time on at work - grinding through a multiple-hundred-thousand-line instant-legacy untested Java codebase. This thing might have been considered state-of-the-art ten years ago when it was all about 3-tiered systems putting messages on busses - iff it had been implemented nicely, but it wasn't. As a result, it's a monolithic proliferation of POJO-manipulation, with control flow by exceptions, mutable state throughout, and impossible to test in isolation.

It can take hours to find code that actually "does something", but you have to follow the path(s) all the way down from "the top" just in case there's a bug or "hidden feature" somewhere on the way through the myriad layers with methods that look like this (anonymised somewhat):
  public List getAllFoo(Integer primaryId, Short secondaryId, String detail, Locale locale,
      String timeZone, String category) {

    if (category != null) {
      Map foosMap = ParameterConstants.foosMap;
      if (foosMap != null) {
        category = (foosMap.get(category.toUpperCase()) != null) ? foosMap.get(category.toUpperCase()) : category;
      }
    }
    List values = new ArrayList();
    FooValue searchValue = new FooValue();
    List fooValues = null;
    searchValue.setPrimaryID(primaryId);
    searchValue.setSecondaryId(secondaryId);
    searchValue.setCategory(category);

    try {
      LOGGER.info(CommonAPILoggingConstants.INF_JOBTYPE_GETALL_VALIDATION_COMPLETED);
      fooValues = fooDAO.getFoos(searchValue, detail);
    } catch (FooValidationException e) {
      handleException(e.getErrorId(), e);
    } catch (Exception e) {
      throw new InternalAPIException(UNKNOWN_CODE, e);

    }
    if (FULL.equalsIgnoreCase(detail)) {
      for (FooValue fooValue : fooValues) {
        Bar bar = null;
        try {
          if (StringUtils.isNotBlank(fooValue.getBarID())) {
            bar = barDAO.getBarByBarId(fooValue.getBarID());
            fooValue.setBarName(bar.getBarName());
            fooValue.setBarShortName(bar.getShortName());

            LOGGER.debug(CommonAPILoggingConstants.DBG_JOBTYPE_GETALL_FETCH_BAR_BY_ID,
                                bar.getBarName(),fooValue.getBarID());
          }
        } catch (Exception e) {
          throw new InternalAPIException(UNKNOWN_CODE, e);
        }

        try {
          if (null != bar) {
            if (StringUtils.isNotBlank(bar.getBrandID())) {
              fooValue.setBazID(bar.getBazID());
                            Baz baz = bazDAO.getBazByBazId(fooValue.getBazID());
              LOGGER.debug(CommonAPILoggingConstants.DBG_JOBTYPE_GETALL_FETCH_BAZ,
                                    baz.getName(),fooValue.getBazID());
              fooValue.setBazName(baz.getName());
            }
          }
        } catch (Exception e) {
          throw new InternalAPIException(UNKNOWN_CODE, e);
        }

        FooValue value = filterFooDetails(fooValue);
        values.add(value);
      }
    } else if (BASIC.equalsIgnoreCase(detail)) {

      for (FooValue fooValue : fooValues) {
        FooValue value = new FooValue();
        value.setFooID(fooValue.getFooID());
        value.setJobName(fooValue.getJobName());
        value.setContentTypeName(fooValue.getContentTypeName());
        value.setCategory(fooValue.getCategory());
        value.setIsOneToMany(fooValue.getIsOneToMany());
        values.add(value);
      }
    } else {
      throw new CommonAPIException(INVALID_DETAIL_PARAM,"Detail parameter value invalid");
    }
    return values;
  }
This is everywhere. The lines that get me most annoyed are things like this:
            fooValue.setBarName(bar.getBarName());
            fooValue.setBarShortName(bar.getShortName());
These x.setFoo(y.getFoo()) stanzas can go on for tens of lines. I haven't come across a name for them, so I'll call them POJO Shuffles. They suck the will-to-live out of anyone who has to navigate them as they frequently contain misalignments, micro-adjustments and hard-coding e.g.:
            fooValue.setBarName(bar.getBazName());
            fooValue.setBarShortName("Shortname: " + bar.getShortName());
            fooValue.setBarLongName(bar.getShortName().toUpperCase());
Did you notice:
  • We're actually getting bazName from bar - almost certainly an autocomplete fail, but perhaps not?
  • The "short name" of fooValue will actually be longer than in the source object. Is that important to something?
  • There's a potential NullPointerException when we innocently try and set the "long name" of the fooValue


Then I read this gem of a paragraph from Rich Hickey, which is merely an introduction to the usage of defrecord in the official Clojure documentation, and yet reads like poetry when you've just come from code like the above:

It ends up that classes in most OO programs fall into two distinct categories: those classes that are artifacts of the implementation/programming domain, e.g. String or collection classes, or Clojure's reference types; and classes that represent application domain information, e.g. Employee, PurchaseOrder etc. It has always been an unfortunate characteristic of using classes for application domain information that it resulted in information being hidden behind class-specific micro-languages, e.g. even the seemingly harmless employee.getName() is a custom interface to data. Putting information in such classes is a problem, much like having every book being written in a different language would be a problem. You can no longer take a generic approach to information processing. This results in an explosion of needless specificity, and a dearth of reuse.
Rich Hickey

Tuesday, 13 September 2011

Extending your SEP field

More anti-complexity ravings

Once you've got over your Complexity "Hump" things become a lot clearer.


The "good programmer == lazy programmer" thing is merely a starting point to a wider world of Using Other People's Stuff. In university this was A Bad Thing, but in the real world, call it "leveraging" and you're on your way to an MBA.


It can be succinctly summarised as Maximising Your SEP - nothing to do with getting a higher Google rank - I'm talking the Somebody Else's Problem. The SEP is the secret to a slender, simple IT solution. The whole "Cloud Computing" hype-bubble is actually SEP translated to the machine-room and its associated sysadmins.


With judicious use of SEP, your "in-house" component is the smallest possible intersection of other peoples' work - be it libraries, frameworks, or servers. Sprinkle in your domain model and you're done.


Moving your infrastructure "to the cloud" is totally hot right now - but how many development shops are actively trying to offload their code too?