bjhuffine
bjhuffine

Reputation: 934

Check type with boolean property?

I'm very aware of type checking, but have found myself in a unique situation and I'm beginning to question whether I'm within best practices. Hopefully the veteran comments will give me some direction and things worth thinking more deeply about. And, to be honest, it's not that what I have will not work, but as I'm making some other changes I'm wondering what the pitfalls might be and whether I should change tactics. There doesn't seem to be a lot out there (in fact I havent' seen anything as basic type checking takes the majority of the search results).

I have a situation where I'm developing a bill of material interface system. In this system, the following class diagram applies:

enter image description here This is generically speaking, but the point here is that there are only three concrete types worth concerning. Because it's easy to set property values in the constructors of the objects, I had defined (generically speaking again of course) the IMaterial interface as so:

public interface IMaterial
{
    bool IsCommodity { get; }

    bool IsAssembly { get; }

    bool IsUnclassified { get; }

    ...
}

Originally, the thought was that the object graph has very little room to change, performance is improved through a preset boolean value, and I don't have to worry about breaking various other principles by type checking concrete types.

So for example, I can do this...

bool hasCommodities = materialCollection.Any(item => item.IsCommodity);
bool hasAssemblies = materialCollection.Any(item => item.IsAssembly);
bool hasDescriptionOnly = materialCollection.Any(item => item.IsUnclassified);

or this...

if (bomMaterial.IsAssembly)
{
    symbol = new BomAssemblySymbol();
}
else
{
    symbol = new BomItemSymbol();
}

instead of this...

bool hasCommodities = materialCollection.Any(item => item is ClassifiedItem);
bool hasAssemblies = materialCollection.Any(item => item is Assembly);
bool hasDescriptionOnly = materialCollection.Any(item => item is UnclassifiedItem);

or this...

if (bomMaterial is Assembly)
{
    symbol = new BomAssemblySymbol();
}
else
{
    symbol = new BomItemSymbol();
}

So in my case, the interface's use of properties means less dependency on concrete types in the implementation detail. But then again, it begs the question, what if another type does come along? What's the best answer here? And is there a pattern that maybe I'm overlooking and should be considering for this? And if anyone is wondering why the consuming code cares, it's because with the CAD system, there is a single command the user interacts with that in turn leverages these objects. I can't create separate commands for them just because of the single line of code difference. Update Here's a more complete example showing how the CAD-side seems to bottle neck processes. The TryGetMaterialInformation() method prompts the user in the CAD system for specific input. The SymbolUtility.InsertSymbol() method just wraps a common set of user prompts for inserting any symbol and then inserts it.

public override void Execute()
{
    IMaterial bomMaterial = null;
    bool multipleByReference = false;
    Editor ed = Application.DocumentManager.MdiActiveDocument.Editor;

    if (!TryGetMaterialInformation(out bomMaterial, out multipleByReference))
    {
        ed.WriteMessage("\nExiting command.\n");
        return;
    }

    IBlockSymbol symbol;
    if (bomMaterial.IsAssembly)
    {
        symbol = new BomAssemblySymbol();
    }
    else
    {
        symbol = new BomItemSymbol();
    }

    if (multipleByReference)
    {
        SymbolUtility.InsertMultipleByReferenceSymbol(symbol, bomMaterial);
    }
    else
    {
        SymbolUtility.InsertSymbol(symbol, bomMaterial);
    }
}

From SymbolUtility

    internal static void InsertSymbol(IBlockSymbol symbol, IMaterial material)
    {
        ICADDocumentDTO document = new CADDocumentDTO();
        Editor ed = document.ActiveDocument.Editor;

        //Get the insert point
        Point3d insertPoint = Point3d.Origin;
        if (!CommandUtility.TryGetPoint("Select insert point: ", out insertPoint))
        {
            ed.WriteMessage("\nExiting command.\n");
            return;
        }

        //Insert the object    
        using (ISystemDocumentLock documentLock = document.Lock())
        {
            CreateSymbolDefinition(symbol, document);

            symbol.Insert(insertPoint, material, document);
        }
    }

Upvotes: 1

Views: 306

Answers (1)

Scott Hannen
Scott Hannen

Reputation: 29212

If you have properties like IsCommodity, IsAssembly, and IsClassified, they should describe some sort of logical property that can be ascribed to an instance. They should not tell the consumer what the concrete type is.

The reason is that a consumer of IMaterial should neither know nor need to know about any concrete type that implements IMaterial.

If those properties actually indicate the concrete types, then all those properties accomplish is type checking, and they will lead to casting objects back to their concrete types, which defeats the purpose of creating an abstraction (interface.)

It looks that way to me since you're considering the properties as a direct alternative to type checking.

The alternative is that instead of the consumer looking at the class properties and deciding what to do or not to with the class, the consumer just tells the class what do to (calling a method) and the implementation of the class itself determines how to carry that out.

Upvotes: 1

Related Questions