1
\$\begingroup\$

I have figures - TriangleItem and CircleItem:

public abstract class BaseItem
{
  protected int _id;
  protected string _description;
}

public sealed class TriangleItem : BaseItem
{
  public int TriangleId { get { return _id; } set { _id = value; } }
  public string TriangleDescription { get { return _description; } set { _description = value; } }
}

public sealed class CircleItem : BaseItem
{
  public int CircleId { get { return _id; } set { _id = value; } }
  public string CircleDescription { get { return _description; } set { _description = value; } }
}

And I have figure collections TriangleBox and CircleBox:

public abstract class BaseBox
{
  public string ItemsXml { get; set; }
  public string Descriptions { get; set; }
  public static abstract BaseBox Boxing( BaseItem[] items );
  public abstract void Unboxing();
}

public sealed class TriangleBox : BaseBox
{
  public static override BaseBox Boxing( BaseItem[] items )
  {
    string[] ids = items.ToList().Cast<TriangleItem>().Select( i => i.TriangleId.ToString() ).ToArray();
    string[] descriptions = items.ToList().Cast<TriangleItem>().Select( i => i.TriangleDescription ).ToArray();

    return new TriangleBox()
    {
      ItemsXml = Util.ToXml( ids ),
      Descriptions = descriptions.Aggregate( ( d, next ) => next + ", " + d )
    };
  }

  public TriangleItem[] Unboxing()
  {
    return Util.FromXml( ItemsXml ).Select( i => new TriangleItem { TriangleId = int.Parse( i ), TriangleDescription = i } ).ToArray();
  }
}

public sealed class CircleBox : BaseBox
{
  public static override BaseBox Boxing( BaseItem[] items )
  {
    string[] ids = items.ToList().Cast<CircleItem>().Select( i => i.CircleId.ToString() ).ToArray();
    string[] descriptions = items.ToList().Cast<CircleItem>().Select( i => i.CircleDescription ).ToArray();

    return new CircleBox()
    {
      ItemsXml = Util.ToXml( ids ),
      Descriptions = descriptions.Aggregate( ( d, next ) => next + ", " + d )
    };
  }

  public CircleItem[] Unboxing()
  {
    return Util.FromXml( ItemsXml ).Select( i => new CircleItem { CircleId = int.Parse( i ), CircleDescription = i } ).ToArray();
  }
}

As you can see, I have almost the same method implementations - Boxing and Unboxing. I use these methods as follows:

  public BaseBox Do( BaseItem[] items )
  {
    BaseBox result = null;

    if( items is CircleItem[] )
      result = CircleBox.Boxing( items );
    else if( items is TriangleItem[] )
      result = TriangleBox.Boxing( items );

    return result;
  }

  public TriangleItem[] GetTriangles()
  {
    TriangleBox tb = DB.GetTriangleBox(); // From database
    return tb.Unboxing();
  }

  public CircleItem[] GetCircles()
  {
    CircleBox cb = DB.GetCircleBox(); // From database
    return cb.Unboxing();
  }

Can I changle something to have only one Boxing and Unboxing implementations? What would you change in this code? What do you dislike (except catch null method's parameters)? AOP and Generics are OK if need.

In Do-method I have to know items's type. And I have to return empty TriangleBox or CircleBox if items.Length is 0. How I have to write my Do-method with this case?

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

I assume you really need to distinguish Circle and Triangle. Any way you can use auto-properties of base class.

public abstract class BaseItem
{
    public int Id { get; set; }
    public string Description { get; set; }
}

public sealed class TriangleItem : BaseItem
{
    // Another TriangleItem class members
}

public sealed class CircleItem : BaseItem
{
    // Another CircleItem class members
}

Boxing mehtods differ in type of object they return, which is subtype of BaseBox though. That means you can merge them and delegate particular boxes creation to subclasses.

public abstract class BaseBox
{
    public string ItemsXml { get; set; }
    public string Descriptions { get; set; }

    public BaseBox Boxing(BaseItem[] items)
    {
        var ids = items.ToList().Select(i => i.Id.ToString()).ToArray();
        var descriptions = items.ToList().Select(i => i.Description.ToString()).ToArray();
        var box = CreateBox(Util.ToXml(ids), descriptions.Aggregate((d, next) => next + ", " + d));
        return box;
    }

    public BaseItem[] Unboxing()
    {
        return Util.FromXml(ItemsXml).Select(i => CreateItem(int.Parse(i), i)).ToArray();
    }

    protected abstract BaseBox CreateBox(string itemsXml, string descriptions);

    protected abstract BaseItem CreateItem(int id, string description);
}

The same can be done for unboxing and BaseItem.

public sealed class TriangleBox : BaseBox
{
    protected override BaseBox CreateBox(string itemsXml, string descriptions)
    {
        return new TriangleBox { ItemsXml = itemsXml, Descriptions = descriptions };
    }

    protected override BaseItem CreateItem(int id, string description)
    {
        return new TriangleItem { Id = id, Description = description };
    }
}

public sealed class CircleBox : BaseBox
{
    protected override BaseBox CreateBox(string itemsXml, string descriptions)
    {
        return new CircleBox { ItemsXml = itemsXml, Descriptions = descriptions };
    }

    protected override BaseItem CreateItem(int id, string description)
    {
        return new CircleItem { Id = id, Description = description };
    }
}

Update: Boxing method should be void. And probably renamed to 'DoBoxing'.

public abstract class BaseBox
{
    ...

    public void Boxing(BaseItem[] items)
    {
        var ids = items.ToList().Select(i => i.Id.ToString()).ToArray();
        var descriptions = items.ToList().Select(i => i.Description.ToString()).ToArray();
        this.ItemsXml = Util.ToXml(ids);
        this.Descriptions = descriptions.Aggregate((d, next) => next + ", " + d);
    }

    ...
}

And called as:

    public BaseBox Do(BaseItem[] items)
    {
        ...

        if (result != null)
        {
            result.Boxing(items);
        }

        return result;
    }
\$\endgroup\$
2
  • 1
    \$\begingroup\$ public BaseBox Boxing(BaseItem[] items) method looks really wierd. Box creating another box? What does that even mean? It should be refactored to void method or a constructor (or both). I would also suggest extracting interface from ItemBase. Using references to abstract classes should be avoided, when possible, imho. \$\endgroup\$
    – Nikita B
    Aug 22, 2013 at 11:31
  • \$\begingroup\$ Nike, agree. I just answered author's initial question on how to avoid multiple Boxing method implementation. But you are right, we should use object + void method here (instead of author's 'static'). I'll make an update... \$\endgroup\$
    – IgorS
    Aug 22, 2013 at 13:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.