HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

MP4 Parser: Reducing code duplication

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mp4parserduplicationreducingcode

Problem

I'm developing an MP4 parser. Each class represents each box in MP4 file, and every box class derives from base box class.
The problem is that below code is duplicated on every box class:

private ulong MyDataSize;
    public override ulong BoxContentEndOffset
    {
        get { return base.BoxContentEndOffset + this.MyDataSize; }
    }

    protected new ParsingState MyParsingState { get; set; }
    public override bool IsParsingDone
    {
        get { return this.MyParsingState.IsParsingDone(); }
    }
    public override bool CanContinueParsing
    {
        get { return ((base.MyParsingState.IsParsingDone() ||
         base.MyParsingState.CanContinueParsing()) && this.MyParsingState.CanContinueParsing()); }
    }


Also, each box-container class has duplicated code like below. (AsReadOnly is an extension method here)

```
public sealed class SampleTableBox : SimpleContainerBoxBase, IActualElement
{
private static readonly IElementType[] _ImplementedElementTypes
= { BoxElementType.Get("stbl") };
public ICollection> ImplementedElementTypes
{
get { return _ImplementedElementTypes; }
}

private static readonly ReadOnlyCollection _PossibleParents
= new ReadOnlyCollection(
new IElementType[]
{
BoxElementType.Get("minf")
});
public override ICollection PossibleParents
{
get { return _PossibleParents; }
}

private static ReadOnlyDictionary, Tuple> _RequiredElements
= new ReadOnlyDictionary, Tuple>(
new Dictionary, Tuple>()
{
{ new [] { BoxElementType.Get("stsd") }.AsReadOnly(), new Tuple(1, 1) },
{ new [] { BoxElementType.Get("stts") }.AsReadOnly(), new Tuple(1, 1) },
{ new [] { BoxElementType.Get("stsz"), BoxElementType.Get("stz2") }.AsReadOnly(), new Tuple(1, 1) },
{ new [] { BoxElementType.Get("stsc") }.AsReadOnly(), new Tuple(1, 1) },

Solution

If I understand this correctly, you shouldn't use inheritance here, use composition instead.

A child box is not a kind of the parent box, it's contained within the parent box. You treat boxes that can contain multiple child boxes this way, I don't see why boxes that contain only a single child should be any different.

This way, all the duplicate logic will be in the base class (which will now be a real base class, not a parent box), so it will exist only once.

new ReadOnlyDictionary, Tuple>(
    new Dictionary, Tuple>()
    {
        { new [] { BoxElementType.Get("stsd") }.AsReadOnly(), new Tuple(1, 1) },
        { new [] { BoxElementType.Get("stts") }.AsReadOnly(), new Tuple(1, 1) },
        { new [] { BoxElementType.Get("stsz"), BoxElementType.Get("stz2") }.AsReadOnly(), new Tuple(1, 1) },
        { new [] { BoxElementType.Get("stsc") }.AsReadOnly(), new Tuple(1, 1) },
        { new [] { BoxElementType.Get("stco"), BoxElementType.Get("co64") }.AsReadOnly(), new Tuple(1, 1) }
    }


When you have code that has as much repetition as this, consider writing a helper function that will take care of it:

public static ReadOnlyDictionary AsReadOnly(
    this IDictionary input)
{
    return new ReadOnlyDictionary(input);
}

private static ReadOnlyDictionary, Tuple>
    CreateRequiredElementsDictionary(Dictionary> input)
{
    return input.ToDictionary(
        kvp =>
            (ICollection)kvp.Key
                .Select(BoxElementType.Get)
                .ToArray()
                .AsReadOnly(),
        kvp => kvp.Value)
        .AsReadOnly();
}

…

CreateRequiredElementsDictionary(
    new Dictionary>
    {
        { new[] { "stsd" }, Tuple.Create(1, 1) },
        { new[] { "stts" }, Tuple.Create(1, 1) },
        { new[] { "stsz", "stz2" }, Tuple.Create(1, 1) },
        { new[] { "stsc" }, Tuple.Create(1, 1) },
        { new[] { "stco", "co64" }, Tuple.Create(1, 1) }
    })


Or even better, write a custom collection that allows you to use better collection initializer:

class RequiredElementsDictionary : Dictionary, Tuple>
{
    // TODO: better parameter names
    public void Add(IEnumerable strings, int firstInt, int secondInt)
    {
        Add(
            strings.Select(BoxElementType.Get).ToArray().AsReadOnly(),
            Tuple.Create(firstInt, secondInt));
    }
}

…

new RequiredElementsDictionary
{
    { new[] { "stsd" }, 1, 1 },
    { new[] { "stts" }, 1, 1 },
    { new[] { "stsz", "stz2" }, 1, 1 },
    { new[] { "stsc" }, 1, 1 },
    { new[] { "stco", "co64" }, 1, 1 }
}.AsReadOnly()


Or maybe also switch the order of parameters, which lets you use params:

class RequiredElementsDictionary : Dictionary, Tuple>
{
    // TODO: better parameter names
    public void Add(int firstInt, int secondInt, params string[] strings)
    {
        Add(
            strings.Select(BoxElementType.Get).ToArray().AsReadOnly(),
            Tuple.Create(firstInt, secondInt));
    }
}

…

new RequiredElementsDictionary
{
    { 1, 1, "stsd" },
    { 1, 1, "stts" },
    { 1, 1, "stsz", "stz2" },
    { 1, 1, "stsc" },
    { 1, 1, "stco", "co64" }
}.AsReadOnly()

Code Snippets

new ReadOnlyDictionary<ICollection<IElementType>, Tuple<int, int>>(
    new Dictionary<ICollection<IElementType>, Tuple<int, int>>()
    {
        { new [] { BoxElementType.Get("stsd") }.AsReadOnly<IElementType>(), new Tuple<int, int>(1, 1) },
        { new [] { BoxElementType.Get("stts") }.AsReadOnly<IElementType>(), new Tuple<int, int>(1, 1) },
        { new [] { BoxElementType.Get("stsz"), BoxElementType.Get("stz2") }.AsReadOnly<IElementType>(), new Tuple<int, int>(1, 1) },
        { new [] { BoxElementType.Get("stsc") }.AsReadOnly<IElementType>(), new Tuple<int, int>(1, 1) },
        { new [] { BoxElementType.Get("stco"), BoxElementType.Get("co64") }.AsReadOnly<IElementType>(), new Tuple<int, int>(1, 1) }
    }
public static ReadOnlyDictionary<TKey, TValue> AsReadOnly<TKey, TValue>(
    this IDictionary<TKey, TValue> input)
{
    return new ReadOnlyDictionary<TKey, TValue>(input);
}

private static ReadOnlyDictionary<ICollection<IElementType>, Tuple<int, int>>
    CreateRequiredElementsDictionary(Dictionary<string[], Tuple<int, int>> input)
{
    return input.ToDictionary(
        kvp =>
            (ICollection<IElementType>)kvp.Key
                .Select(BoxElementType.Get)
                .ToArray<IElementType>()
                .AsReadOnly(),
        kvp => kvp.Value)
        .AsReadOnly();
}

…

CreateRequiredElementsDictionary(
    new Dictionary<string[], Tuple<int, int>>
    {
        { new[] { "stsd" }, Tuple.Create(1, 1) },
        { new[] { "stts" }, Tuple.Create(1, 1) },
        { new[] { "stsz", "stz2" }, Tuple.Create(1, 1) },
        { new[] { "stsc" }, Tuple.Create(1, 1) },
        { new[] { "stco", "co64" }, Tuple.Create(1, 1) }
    })
class RequiredElementsDictionary : Dictionary<ICollection<IElementType>, Tuple<int, int>>
{
    // TODO: better parameter names
    public void Add(IEnumerable<string> strings, int firstInt, int secondInt)
    {
        Add(
            strings.Select(BoxElementType.Get).ToArray<IElementType>().AsReadOnly(),
            Tuple.Create(firstInt, secondInt));
    }
}

…

new RequiredElementsDictionary
{
    { new[] { "stsd" }, 1, 1 },
    { new[] { "stts" }, 1, 1 },
    { new[] { "stsz", "stz2" }, 1, 1 },
    { new[] { "stsc" }, 1, 1 },
    { new[] { "stco", "co64" }, 1, 1 }
}.AsReadOnly()
class RequiredElementsDictionary : Dictionary<ICollection<IElementType>, Tuple<int, int>>
{
    // TODO: better parameter names
    public void Add(int firstInt, int secondInt, params string[] strings)
    {
        Add(
            strings.Select(BoxElementType.Get).ToArray<IElementType>().AsReadOnly(),
            Tuple.Create(firstInt, secondInt));
    }
}

…

new RequiredElementsDictionary
{
    { 1, 1, "stsd" },
    { 1, 1, "stts" },
    { 1, 1, "stsz", "stz2" },
    { 1, 1, "stsc" },
    { 1, 1, "stco", "co64" }
}.AsReadOnly()

Context

StackExchange Code Review Q#59858, answer score: 3

Revisions (0)

No revisions yet.