Saturday, August 23, 2014

What you know and have impacts your design



Description

You can only design from what you know and/or have available to you. Here's a short video showing the evolution of a electric switch design based on new information.

Transcript

Shop equipment frequently have two push-button toggle switches to turn things on and off. I recently worked on a project where I wanted to replicate this behavior to control an appliance and do it inexpensively. My hope was to get by with junk bin parts.

Looking around at my spare parts, I found a project box, a couple momentary switches and a relay with contacts rated above what I wanted to control. So I started with these items.

It was a double-pole relay. My thought was to use one pole of the relay to hold itself energized and the other pole to switch my device. So, one switch energizes the relay and the relay then holds itself closed. This should allow me to turn on the device.

I figured a transistor could be placed between supply voltage and the relay's coil with the base wired to the supply voltage. The "off" switch could connect to the base of the transistor and ground. In this configuration, the transistor would normally be on allowing, the relay to be energized. But when the button is pressed, the transistor would turn off the power to the coil, switching the relay off.

I wired it all together and it worked!

The relay circuit was too big to fit in the project box though, so I wired the switches through a four conductor wire and put the circuit in a separate box next to the switched item.

Even though it worked, I wasn't too wild about having high voltage and low voltage running through the different poles of the relay. It seemed like a pretty bad hack.

While I worked on other aspects of the project, I ran across something called a "PowerSwitch Tail." This looks like a short extension cord but it has a twist. It has a solid-state relay built into it. Normally the outlets are turned off, but place a low voltage across these connectors and the outlets will turn on.

This seemed like a much better solution than my relay hack.

So I completely rethought my approach.

I could have simply replaced the high voltage side of the relay circuit with a low voltage source going to the power tail's connectors. This would work, but now the relay seemed way overkill. I had a low voltage circuit controlling another low voltage circuit through a relay.

Instead I replaced the relay based circuit with a flip-flop circuit. These can be built from discrete components, but I had a quad NAND gate chip in the parts box and this would shrink the size significantly.

I wired up the new circuit and it worked. A much cleaner approach.

As I worked on other aspects of the project, I had a realization: that flip-flop circuit was small enough it could fit in the project box with the switches. If I reconfigured the 4-conductor wire to have voltage, ground and signal, I could move the circuit to the box.

In the end, I did this as it allows future changes to the switching mechanism. Right now I have the two push button toggle switch configuration. But at some point, I'm thinking I want to have a current detecting switch to turn one appliance on and off based on another appliance being on or off. This is easily done with the new configuration but wasn't so convenient with the old.

So, that's my story about how this design evolved over time as I discovered new resources and thought about the problem at a deeper level.

Catch you next time.

Wednesday, April 9, 2014

Are enumerations evil?

Or, How to refactor an enumeration to multiple classes

I ran across this article about how to refactor a switch statement to a data structure in JavaScript a while ago. This is not a terrible refactoring technique. I've used something similar with dictionaries in C# where the key was an enumeration and the value was an Action (or Func). It can be a minor refactoring to a single method that can help clean-up the code. Then this week I had a conversation with a fellow developer about the appropriateness of an enumeration in existing code from a couple different libraries he was reading.

This led me to wonder afresh if perhaps enumerated types aren't something that we as the development community should treat as an anti-pattern. In the days before class types, they were a means for the compiler to force integers to known values and provide more readable code for the developer. The compiler could check values better at compile time and catch usage errors earlier. I have worked on languages without them and embraced them when they became available. At that point they were a good thing.

Many times enumerations are used to change behavior of some code based on a variable's value. This is usually done through switch or if-then-else statements. Or, as discussed above, using some sort of dictionary like structure to store behavior associated with a specific enumerated value.

The problem is this pattern tends to get replicated within the class containing the enumerated variable. The class using the enumeration needs to do different things based on the various values the variable can have and these different things spread throughout the class (or classes) using the enumeration. Then, when a value is added to the enumeration, each place the variable is tested needs to be updated to handle the new value.

This can lead to a number of problems. With behavior based on the enum scattered throughout classes, the intent can be both obfuscated and duplicated. When adding a value to the enumeration, it's easy to miss a place that needs new behavior, introducing hard to detect bugs. The code is fragile in the face of changes. And because the state and associated behavior associated with the enumeration is mixed in with the class (or classes) using it, violation of the Single Responsibility Principle frequently is seen.

Refactoring to a data structure as mentioned above is a good first step. It helps address a number of the problems. But in today's object oriented world, there is a better way. In many (perhaps most) cases it's better to refactor to multiple classes, one for each value in the enumeration. Fortunately, this is fairly easily done.

Here's the original starting code:
enum SomeItemEnum = { one, two, three };
class A
{
  SomeItemEnum someItemValue;
  void SomeItemUser()
  {
    switch(someItemValue)
    {
      case one:
        // Some complicated code for case one
        break;
      case two:
        // Some complicated code for case two
        break;
      case three:
        // Some complicated code for case three
        break;
    }
  }
}
First, a base class is created as an ancestor for the enumeration.
enum SomeItemEnum = { one, two, three };

class SomeItemEnumBase
{
}

class A
{
  SomeItemEnum someItemValue;
  void SomeItemUser()
  {
    switch(someItemValue)
    {
      case one:
        // Some complicated code for case one
        break;
      case two:
        // Some complicated code for case two
        break;
      case three:
        // Some complicated code for case three
        break;
    }
  }
}
Then, each place where something needs to be done based on the value of the enumeration, a method is added to the class. If there is no default behavior for a method, it should be abstract, otherwise it should be virtual.
enum SomeItemEnum = { one, two, three };

abstract class SomeItemEnumBase
{
  abstract void ComplicatedCode();
}

class A
{
  SomeItemEnum someItemValue;
  void SomeItemUser()
  {
    switch(someItemValue)
    {
      case one:
        // Some complicated code for case one
        break;
      case two:
        // Some complicated code for case two
        break;
      case three:
        // Some complicated code for case three
        break;
    }
  }
}
A child class should be made for each value of the enumeration with the value specific behavior moved to the appropriate overridden method.
enum SomeItemEnum = { one, two, three };

abstract class SomeItemEnumBase
{
  abstract void ComplicatedCode();
}

class SomeItemEnumOne : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case one
  }
}

class SomeItemEnumTwo : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case two
  }
}

class SomeItemEnumThree : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case three
  }
}

class A
{
  SomeItemEnum someItemValue;
  void SomeItemUser()
  {
    switch(someItemValue)
    {
      case one:
        // Some complicated code for case one
        break;
      case two:
        // Some complicated code for case two
        break;
      case three:
        // Some complicated code for case three
        break;
    }
  }
}
Change the type of the class' variable from the enumeration to the base type.
enum SomeItemEnum = { one, two, three };

abstract class SomeItemEnumBase
{
  abstract void ComplicatedCode();
}

class SomeItemEnumOne : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case one
  }
}

class SomeItemEnumTwo : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case two
  }
}

class SomeItemEnumThree : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case three
  }
}

class A
{
  SomeItemEnumBase someItemValue;
  void SomeItemUser()
  {
    switch(someItemValue)
    {
      case one:
        // Some complicated code for case one
        break;
      case two:
        // Some complicated code for case two
        break;
      case three:
        // Some complicated code for case three
        break;
    }
  }
}
Instead of setting discrete values on the enumerated variable, now a class instance of the appropriate type can be set.

Old method:
someItemValue = SomeItemEnum.one;
someItemValue = SomeItemEnum.two;
New method:
someItemValue = new SomeItemEnumOne();
someItemValue = new SomeItemEnumTwo();
All the switch/if-else statements can now be changed to simple method calls that have polymorphic behavior based on the class type.
enum SomeItemEnum = { one, two, three };

abstract class SomeItemBase
{
  abstract void ComplicatedCode();
}

class SomeItemOne : SomeItemBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case one
  }
}

class SomeItemTwo : SomeItemBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case two
  }
}

class SomeItemThree : SomeItemBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case three
  }
}

class A
{
  SomeItemEnumBase someItemValue;
  void SomeItemUser()
  {
    someItemValue.ComplicatedCode();
  }
}
And finally, the unused enumeration declaration can be removed.
enum SomeItemEnum = { one, two, three };

abstract class SomeItemEnumBase
{
  abstract void ComplicatedCode();
}

class SomeItemEnumOne : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case one
  }
}

class SomeItemEnumTwo : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case two
  }
}

class SomeItemEnumThree : SomeItemEnumBase
{
  override void ComplicatedCode()
  {
    // Some complicated code for case three
  }
}

class A
{
  SomeItemEnumBase someItemValue;
  void SomeItemUser()
  {
    someItemValue.ComplicatedCode();
  }
}
Now, when a new value is added, only places that actually care about the new value specifically (e.g. where its value is set) need to be touched. As a bonus, the compiler will complain about any unimplemented abstract functions, ensuring all required behavior is implemented.

Finally, admittedly, in this toy example, the final result is more complicated and obtuse than the original. If production code is as simple as this example, then it doesn't make sense to make this change. However, when class A is larger and more complex, the increased clarity and robustness of the code can be significant. In my experience, the latter is more typical than the former and so in general, my conclusion is enumerations border on evil.

Friday, March 14, 2014

Weird compiler error: C2360: initialization is skipped by label

I ran into a new error the other day that was non-obvious at first glance.

I had code structured something like (greatly simplified by way of example):
void MyFunction()
{
     int a = GetValueOfA();
     switch(a)
     {
          case 1:
               int b = 2;
               DoSomething(b);
               break;
          case 2:
               int c = 3;
               DoSomethingElse(c);
               break;
     }
}
This gave me an error on the second case "error C2360: initialization of 'b' is skipped by 'case' label." *

What?!? This is one of those cases where the message is technically accurate after you understand what it says but utterly useless to explain what is going on or what the solution is at first glance. Or perhaps I'm slow. I stared at this for a bit before I comprehended what it was trying to tell me.

The root of the issue is that, while the case statements appear to be in their own scope, they aren't. (I'm sure I knew this at some point, but the memory was overridden long ago. Or perhaps I have too many languages floating around in my head.) The scope for variables inside a switch statement is all the cases, not just the current case. Therefore, in the code above, variables b and c are available anywhere inside the switch statement. The error says b is defined, but may not be initialized properly, when a = 2.

All the solutions involve changing the scope of b and c. There are a couple immediately obvious solutions:

1) Put braces around the contents of the case statements to limit their scope.
void MyFunction()
{
     int a = GetValueOfA();
     switch(a)
     {
          case 1:
               {
                    int b = 2;
                    DoSomething(b);
               }
               break;
          case 2:
               {
                    int c = 3;
                    DoSomethingElse(c);
               }
               break;
     }
}
2) Put the contents of the case statements in their own functions, another way of limiting their scope.
void PerformCase1()
{
     int b = 2;
     DoSomething(b);
}

void PerformCase2()
{
     int c = 3;
     DoSomethingElse(c);
}

void MyFunction()
{
     int a = GetValueOfA();
     switch(a)
     {
          case 1:
               PerformCase1();
               break;
          case 2:
               PerformCase2();
               break;
     }
}
3) Move the declaration to before the switch statement and don't do initialization/declaration on the same line.
void MyFunction()
{
     int b, c;
     int a = GetValueOfA();
     switch(a)
     {
          case 1:
               b = 2;
               DoSomething(b);
               break;
          case 2:
               c = 3;
               DoSomethingElse(c);
               break;
     }
}
Another option would be to change the structure of the code such that the switch isn't necessary. In general, this would be my favored solution although in this specific case of legacy code, that would have involved more work than the budget allowed.

* Actually my notes say I also got a second error on the switch stating "transfer of control bypasses initialization of variable b" but I could not reproduce this one. Perhaps I simplified too much. Or perhaps it was a different version of the compiler. Or perhaps different option settings. Or something else entirely.

Monday, February 24, 2014

Constant integer values and multi-language COM interop

I recently moved some code from a legacy C++ application into a COM library for more general use. The original code was duplicated a couple times in different C++ applications. Then a need arose to use this code in C#. To clean up both the duplication and make it available to .Net applications, we decided to untangle it from the original applications and move it into its own COM library.

One of the issues I ran into that took a bit to figure out involved constant values.

The old C code had several things defined as integers with associated constant definitions to handle bit-mapped values. Some might argue these should be converted to enumerations but we wanted to minimize changes to existing code structure and so decided to keep them as integer constants. The issue with this was how to move them to a common location for all COM clients to access.

It took a bit of research to find the answer as I didn't find a complete answer in one place. Hopefully this article will help fill that gap.

IDL files can have #define statements. This works for C code. The IDL compiler maps these as corresponding #defines in the corresponding intermediate .h files. The problem is when .Net creates an Interop assembly, they are ignored. This means they don't exist for use in .Net languages.

Next I used const definitions in the library. Again, this worked for C code but the Interop assembly in the .Net world did not have them.

I moved the const definitions into an interface. Still, C was perfectly happy but the Interop ignored them.

I search around some more. I ran into some forum discussions that indicated it was impossible.

Finally I found a reference to something that pointed me in the correct direction. IDL files can have the concept of modules, something I hadn't run into before. I put the const declaration in a module section in the IDL file. In the intermediate .h file for C++, these are simply constants and callers are still happy. But in this case, when the .Net Interop file is created they are not ignored. The Interop generator maps them to an abstract class named with the name of the module and the constants become static consts inside the class. This makes them available for any .Net user to access.

So, the IDL file ended up looking like this:
library SomeComLibrary
{
    module Constants
    {
        const DWORD UniqueValue = 0xFFFFFFFF;
    }
}
C's intermediate .h file looks like this:
#ifndef __Constants_MODULE_DEFINED__
#define __Constants_MODULE_DEFINED__
/* module Constants */
const DWORD UniqueValue = 0xffffffff;
#endif /* __Constants_MODULE_DEFINED__ */
And the .Net Interop file looks like this:
namespace SomeComLibrary
{
    public abstract class Constants
    {
        public const uint UniqueValue = -1;
    }
}
The solution ended up being quite easy, but finding it was a bit of a challenge.

Things I searched for trying to find a solution to this problem included:
  • idl const not in .net
  • idl const not in interop
  • idl const not in C# interop
  • midl keywords
  • idl const .net