3
\$\begingroup\$

I was reading about the MIDI spec and I challenged myself to implement a function to convert an int32 to a variable length quantity uint32.

I didn't look it up or anything so there might be a way more efficient or tricky way to get the same result, but as far as doing what I set out to accomplish the following code works.

I'm interested to know if anyone can significantly simplify or improve the algorithm that does the conversion, public static uint CalculateEncodedQuantity(int q).

I included the whole thing in case anyone wanted to paste it in to VS.

Specification: enter image description here

Tests:

using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace VariableLengthRepresentation.Test
{
    [TestClass]
    public class UnitTest1
    {
        [TestMethod]
        public void TestMethod1()
        {
            Assert.AreEqual(0u, new VariableLengthQuantity(0).EncodedQuantity);
            Assert.AreEqual(0x40u, new VariableLengthQuantity(0x40).EncodedQuantity);
            Assert.AreEqual(0x7fu, new VariableLengthQuantity(0x7f).EncodedQuantity);

            //  input:           1000 0000
            // output: 1000 0001 0000 0000
            Assert.AreEqual(0x8100u, new VariableLengthQuantity(0x80).EncodedQuantity);

            //  input: 0010 0000 0000 0000
            // output: 1100 0000 0000 0000
            Assert.AreEqual(0xc000u, new VariableLengthQuantity(0x2000).EncodedQuantity);

            Assert.AreEqual(0xff7fu, new VariableLengthQuantity(0x3fff).EncodedQuantity);
            Assert.AreEqual(0x818000u, new VariableLengthQuantity(0x4000).EncodedQuantity);
            Assert.AreEqual(0xc08000u, new VariableLengthQuantity(0x100000).EncodedQuantity);
            Assert.AreEqual(0xffff7fu, new VariableLengthQuantity(0x1fffff).EncodedQuantity);
            Assert.AreEqual(0x81808000u, new VariableLengthQuantity(0x200000).EncodedQuantity);
            Assert.AreEqual(0xc0808000u, new VariableLengthQuantity(0x8000000).EncodedQuantity);
            Assert.AreEqual(0xffffff7fu, new VariableLengthQuantity(0xfffffff).EncodedQuantity);
        }
    }
}

Implementation:

using System;

namespace VariableLengthRepresentation
{
    public struct VariableLengthQuantity
    {
        public VariableLengthQuantity(int quantity)
        {
            Quantity = quantity;
            EncodedQuantity = CalculateEncodedQuantity(quantity);
        }

        public static uint CalculateEncodedQuantity(int q)
        {
            if (q > 0x0fffffff)
            {
                throw new Exception("Variable length quantity cannot exceed 0x0fffffff.");
            }

            uint n = (uint)q;

            if (n < 128)
                return n;

            var result = new byte[4];

            for (int i = 3; i >= 0; i--)
            {
                result[i] = (byte)(n & 0x7f);

                if(i < 3)
                    result[i] |= 0x80;

                n >>= 7;

                if (n < 1)
                    break;
            }

            if(BitConverter.IsLittleEndian)
                Array.Reverse(result);

            return BitConverter.ToUInt32(result, 0);
        }

        public int Quantity { get; private set; }
        public uint EncodedQuantity { get; private set; }
    }
}
\$\endgroup\$
1
\$\begingroup\$

If an argument doesn't fit in the desired range an ArgumentOutOfRangeException should be thrown instead of an Exception.


    for (int i = 3; i >= 0; i--)
    {
        result[i] = (byte)(n & 0x7f);

        if(i < 3)
            result[i] |= 0x80;

        n >>= 7;

        if (n < 1)
            break;
    }  

This can be rewritten to remove the if(i < 3) by starting the loop at 2. The check if (n < 1) can be placed in the loop condition like

result[3] = (byte)(n & 0x7f);
n >>= 7;  

for (int i = 2; (i >= 0) || (n > 0); i--)
{
    result[i] = ((byte)(n & 0x7f)) | 0x80;
    n >>= 7;
}  

You have some magic numbers in your code. The values 0x0fffffff, 128, 0x7f, 0x80 and 7 should be extracted to descriptive named constants.


Also there isn't explicitly written anything in any guidelines about braces {} for single instruction if statements I would encourage you to always use them to make your code less error prone.


You shouldn't shorten variables / parameter names. It is easier to read if the passed in parameter is named quantity instead of q.


The fact that you are taking care about the Endianess of the system should be stated in a xml documentation.

\$\endgroup\$

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.