1

Description of the environment:

  • I am implementing a (hobbyist) modern engine for (an old) PlayStation 1 video game
  • Graphical data is represented as packets where it can represent either a polygon or a sprite,
    (and they are transformed differently to end up as pixels on screen)
  • All those differents type of packets (18 in total) can in fact be reduced to 2 or 3 types at most (what I've done to reduce code surface), with some flags indicating what's present in them

For information purposes,

This format is more or less a variant of the HMD format by Sony, an efficient low-level format that gets consumed directly by the PlayStation 1 GPU, at the expense of being unfriendly to read/parse, etc ... (actually on the real PlayStation 1, one would emit few system library calls to load it and that was it)

Description of the problem:

  • I need to find a (simple yet effective) way of realizing their content
  • accounting that they can either be a polygon or a sprite

Long story short,

The following code snippets below probably explains better what's the content and the preliminary approach,

Interfaces that we'll work against:

/// <summary>
///     a graphics primitive.
/// </summary>
public interface IPrimitive
{
    bool IsPolygon { get; }
    bool IsSprite { get; }
}

/// <summary>
///     a polygon, i.e. triangle, quad.
/// </summary>
public interface IPolygon : IPrimitive
{
    void GetMesh();
}

/// <summary>
///     a sprite, (internal representation has different meaning than a polygon).
/// </summary>
public interface ISprite : IPrimitive
{
    void GetMesh();
}

Type present in game data:

/// <summary>
///     a packet representing graphical data : a polygon or a sprite.
/// </summary>
public class GraphicsPacket : IPolygon, ISprite
{
    public GraphicsPacket(Stream stream)
    {
        // read packet ...


        // there will be a flag indicating if it's a sprite
        // and therefore should be treated differently
    }

    public bool IsSprite { get; }

    public bool IsPolygon => !IsSprite;

    void IPolygon.GetMesh()
    {
        if (IsSprite)
            throw new InvalidOperationException("Instance is not a polygon");
    }

    void ISprite.GetMesh()
    {
        if (IsPolygon)
            throw new InvalidOperationException("Instance is not a sprite");
    }
}

Test of that whole logic:

public class Test
{
    public void BuildPrimitive([NotNull] IPrimitive primitive)
    {
        if (primitive == null)
            throw new ArgumentNullException(nameof(primitive));

        // obviously here, 'as' cast would always be true for all cases

        var polygon = primitive.IsPolygon;
        if (polygon)
        {
            // do something with it
            ((IPolygon)polygon).GetMesh();

            return;
        }

        var sprite = primitive.IsSprite;
        if (sprite)
        {
            // do something with it
            ((ISprite)sprite).GetMesh();

            return;
        }

        throw new NotSupportedException(nameof(primitive));
    }
}

Question:

Is my approach for treating a graphics packet depending its content a good one ?

i.e.

  • explicit interface implementation
  • naive bool check and throw on invalid state

Strong arguments about considering splitting the concerns in such case,

  • game data is just like that, why change the logic ?
  • (my) modern, object-oriented approach, summarized this as few types (i.e. DRY concept)
  • problem won't really vanish, as those graphics packet will always be either a polygon or a sprite (i.e. why fight against that logic after all ?)

Hope that makes sense to you, otherwise let me know how I can improve the question.

2

I believe you only need one interface that classes for Polygon and Sprite implement. Based on what I see I would make an interface for GraphicsPacket and create Polygon and Sprite classes that implement it's methods.

Then throughout your code you just write it to use GraphicsPacket objects and you don't need to decide numerous times whether it's a Polygon or Sprite.

public interface IGraphicsPacket
{
    public GraphicsPacket(Stream stream);

    public GetMesh()
}

public class  Polygon : IGraphicsPacket
{
    public GraphicsPacket(Stream stream)
    {
        //Polygon implementation
    }

    public GetMesh()
    {
        //Polygon implementation
    }
}

public class  Sprite : IGraphicsPacket
{
    public GraphicsPacket(Stream stream)
    {
        //Sprite implementation
    }

    public GetMesh()
    {
        //Sprite implementation
    }
}

TEST

public class Test
{
    public void BuildPrimitive([NotNull] IGraphicsPacket primitive)
    {
        if (primitive == null)
            throw new ArgumentNullException(nameof(primitive));

        primitive.GetMesh();

        return;
    }
}
  • Okay, your first sentence definitely makes sense. But I don't get it for the second, actually the whole point is to not have sub-classes like Polygon/Sprite as they only differ by flags, could you elaborate ? – Aybe May 18 '17 at 19:57
  • 2
    If they only differs in flags, why don't you just implement a single concrete class? For what is the hierarchy of interfaces? – Laiv May 18 '17 at 20:12
  • 1
    Implementing an interface isn't the same as sub-classing/inheritance. – jdobrzen May 18 '17 at 20:20
  • 2
    Unless the C# spec has changed, you appear to be specifying a constructor in the interface, which you can't do. Nothing wrong with having an abstract base class as well as the interface. The abstract base class implements the constructors you need to create the objects. The interface defines the behavior after creation. – Greg Burghardt May 19 '17 at 12:08
  • 1
    An abstract class with all virtual members is an interface. Usually not much sense in using both @GregBurghardt – RubberDuck Jun 17 '17 at 21:58
0

In fact, you don't respect the DRY principle with your code because you are always explicitly testing the type.

This is a perfect example where someone should use a class hierarchy as already recommended in the other answer by jdobrzen.

The way you avoid code duplication is either by having shared implementation in a base class or in an helper class.

public class PrimitiveBase 
{ 
    /* contains common code, probably with protected */
}

public class Polygone : PrimitiveBase, IPolygone { ... }

Then you should also have a factory method that will create the appropriate object from the stream. One way would be that all your interface derives from IPrimitive and have a factory.

Something like:

public class PrimitiveFactory
{
    IPrimitive CreateFromStream(Stream stream)
    {
        // - Read primitive type from stream
        // - Reset stream position (if necessary)
        // - Create appropriate primitive (might be a switch statement)
        // - If data loading is separate from constructor, then load data
    }
}    

You can use DI containers or some serialization framework if desired...

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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