All your Base - Exercism.io

Question

This is an exercise on the website Exercism. I am self taught trying to learn C# and C++. This is the information from the readme.

Convert a number, represented as a sequence of digits in one base, to any other base.

Implement general base conversion. Given a number in base a, represented as a sequence of digits, convert it to base b.

Note

• Try to implement the conversion yourself. Do not use something else to perform the conversion for you.

About Positional Notation

In positional notation, a number in base b can be understood as a linear combination of powers of b.

The number 42, in base 10, means:

(4 10^1) + (2 10^0)

The number 101010, in base 2, means:

(1 *2^5) + (0 *2^4) + (1 2^3) + (0 2^2) + (1* 2^1) + (0* 2^0)

The number 1120, in base 3, means:

(1 *3^3) + (1 3^2) + (2 3^1) + (0* 3^0)

I think you got the idea!

Yes. Those three numbers above are exactly the same. Congratulations!

Following script is my solution to the exercise.

using System;
using System.Collections.Generic;
using System.Linq;

public static class AllYourBase
{
    private static void RebaseIsException(int inputBase, int[] digits, int outputBase)
    {
        if (digits is null)
            throw new ArgumentNullException(nameof(digits));

        if (inputBase <= 1)
            throw new ArgumentException(nameof(inputBase));

        if (digits.Any(e => e < 0))
            throw new ArgumentException(nameof(digits));

        if (digits.Any(e => e >= inputBase))
            throw new ArgumentException(nameof(inputBase), nameof(digits));

        if (outputBase <= 1)
            throw new ArgumentException(nameof(outputBase));

        if (inputBase <= 0 && outputBase <= 0)
            throw new ArgumentException(nameof(inputBase), nameof(outputBase));
    }

    public static bool RebaseIsDigitsEmptyOrZero(int[] digits) => digits.Sum() == 0;

    public static int[] RebaseSolution(int inputBase, int[] digits, int outputBase)
    {
        int number = 0;
        List<int> list = new List<int>();

        foreach (int i in digits)
            number = number * inputBase + i;

        do
        {
            list.Add(number % outputBase);
        } while ((number /= outputBase) != 0);

        list.Reverse();
        return list.ToArray();
    }

    public static int[] Rebase(int inputBase, int[] digits, int outputBase)
    {
        RebaseIsException(inputBase, digits, outputBase);

        return RebaseIsDigitsEmptyOrZero(digits) ? new int[] { 0 } : RebaseSolution(inputBase, digits, outputBase);
    }
}

Next, is the test case for this exercise.

// This file was auto-generated based on version 2.3.0 of the canonical data.

using System;
using Xunit;

public class AllYourBaseTest
{
    [Fact]
    public void Single_bit_one_to_decimal()
    {
        var inputBase = 2;
        var digits = new[] { 1 };
        var outputBase = 10;
        var expected = new[] { 1 };
        Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Binary_to_single_decimal()
    {
        var inputBase = 2;
        var digits = new[] { 1, 0, 1 };
        var outputBase = 10;
        var expected = new[] { 5 };
        Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Single_decimal_to_binary()
    {
        var inputBase = 10;
        var digits = new[] { 5 };
        var outputBase = 2;
        var expected = new[] { 1, 0, 1 };
        Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Binary_to_multiple_decimal()
    {
        var inputBase = 2;
        var digits = new[] { 1, 0, 1, 0, 1, 0 };
        var outputBase = 10;
        var expected = new[] { 4, 2 };
        Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Decimal_to_binary()
    {
        var inputBase = 10;
        var digits = new[] { 4, 2 };
        var outputBase = 2;
        var expected = new[] { 1, 0, 1, 0, 1, 0 };
        Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Trinary_to_hexadecimal()
    {
        var inputBase = 3;
        var digits = new[] { 1, 1, 2, 0 };
        var outputBase = 16;
        var expected = new[] { 2, 10 };
        Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Hexadecimal_to_trinary()
    {
        var inputBase = 16;
        var digits = new[] { 2, 10 };
        var outputBase = 3;
        var expected = new[] { 1, 1, 2, 0 };
        Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Number_15_bit_integer()
    {
        var inputBase = 97;
        var digits = new[] { 3, 46, 60 };
        var outputBase = 73;
        var expected = new[] { 6, 10, 45 };
        Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Empty_list()
    {
        var inputBase = 2;
        var digits = Array.Empty<int>();
        var outputBase = 10;
        var expected = new[] { 0 };
        Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Single_zero()
    {
        var inputBase = 10;
        var digits = new[] { 0 };
        var outputBase = 2;
        var expected = new[] { 0 };
        Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Multiple_zeros()
    {
        var inputBase = 10;
        var digits = new[] { 0, 0, 0 };
        var outputBase = 2;
        var expected = new[] { 0 };
        Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Leading_zeros()
    {
        var inputBase = 7;
        var digits = new[] { 0, 6, 0 };
        var outputBase = 10;
        var expected = new[] { 4, 2 };
        Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Input_base_is_one()
    {
        var inputBase = 1;
        var digits = new[] { 0 };
        var outputBase = 10;
        Assert.Throws<ArgumentException>(() => AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Input_base_is_zero()
    {
        var inputBase = 0;
        var digits = Array.Empty<int>();
        var outputBase = 10;
        Assert.Throws<ArgumentException>(() => AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Input_base_is_negative()
    {
        var inputBase = -2;
        var digits = new[] { 1 };
        var outputBase = 10;
        Assert.Throws<ArgumentException>(() => AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Negative_digit()
    {
        var inputBase = 2;
        var digits = new[] { 1, -1, 1, 0, 1, 0 };
        var outputBase = 10;
        Assert.Throws<ArgumentException>(() => AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Invalid_positive_digit()
    {
        var inputBase = 2;
        var digits = new[] { 1, 2, 1, 0, 1, 0 };
        var outputBase = 10;
        Assert.Throws<ArgumentException>(() => AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Output_base_is_one()
    {
        var inputBase = 2;
        var digits = new[] { 1, 0, 1, 0, 1, 0 };
        var outputBase = 1;
        Assert.Throws<ArgumentException>(() => AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Output_base_is_zero()
    {
        var inputBase = 10;
        var digits = new[] { 7 };
        var outputBase = 0;
        Assert.Throws<ArgumentException>(() => AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Output_base_is_negative()
    {
        var inputBase = 2;
        var digits = new[] { 1 };
        var outputBase = -7;
        Assert.Throws<ArgumentException>(() => AllYourBase.Rebase(inputBase, digits, outputBase));
    }

    [Fact]
    public void Both_bases_are_negative()
    {
        var inputBase = -2;
        var digits = new[] { 1 };
        var outputBase = -7;
        Assert.Throws<ArgumentException>(() => AllYourBase.Rebase(inputBase, digits, outputBase));
    }
}

I was able to without effort come up with RebaseIsException to pass the test that wants you to throw new ArgumentException. Few other test where passed using RebaseIsDigitsEmptyOrZero. I had to borrow from a few sites to come up with what you see in RebaseSolution. In particular the foreach loop and the do/while loop to get the solution. I kind of stumbled into passing this after a few days of spending a few minutes on it each night, with most the time just in this one function.

Anyhow, I would be grateful and appreciate whatever feedback to help me continue to learn. Thank you!

Answer 1

RebaseIsException()

This is a little strange name for a method. It doesn't tell anything about what happens inside it. A better name would be VerifyInput() or ValidateInput()

---

  if (inputBase <= 0 && outputBase <= 0)
    throw new ArgumentException(nameof(inputBase), nameof(outputBase));

This will never happen, because <= 0 is caught by the previous <= 1

---

public static bool RebaseIsDigitsEmptyOrZero(int[] digits) => digits.Sum() == 0;

Again a little strange name.

Besides that I think your general algorithm should be able to handle the zero-value of any base.

Further: You test against each digit not being negative. Therefore the only way this can return true is if digits only contains zeros or is empty and that is handled by RebaseSolution().

RebaseIsDigitsEmptyOrZero(int[] digits) is all in all an optimization for zero-values, but in fact a bottleneck for all other values, because it iterate through the list one extra time.

---

This:

  if (digits.Any(e => e < 0))
    throw new ArgumentException(nameof(digits));

  if (digits.Any(e => e >= inputBase))
    throw new ArgumentException(nameof(inputBase), nameof(digits));

can be combinded to:

  if (digits.Any(e => e < 0 || e >= inputBase))
    throw new ArgumentOutOfRangeException(nameof(digits));

saving one iteration of the data set (in worst case).

Answer 2

Try to avoid doing sneaky value changes in what is otherwise a logical evaluation. Not that it doesn't work, but condensing these steps into one another detracts from readability.

// Yours

do
{
    list.Add(number % outputBase);
} while ((number /= outputBase) != 0);

// My suggestion

do
{
    list.Add(number % outputBase);
    number /= outputBase;
} while (number != 0);

---

Avoid defaulting to publically accessible methods. In AllYourBase, only Rebase should be public, the others can be made private.

This helps consumers of your static class know which method to call as they can't use the methods they shouldn't call directly.

---

The validation logic can do with some descriptive error messages. You test very specific circumstances but then don't actually report back which part of the validation failed.

Try to provide a meaningful message in your exception, such as base cannot be zero or negative or digit values exceed input base.

---

There also seems to be a disconnect in your validation itself:

if (inputBase <= 1)
    throw new ArgumentException(nameof(inputBase));

if (outputBase <= 1)
    throw new ArgumentException(nameof(outputBase));

if (inputBase <= 0 && outputBase <= 0)
    throw new ArgumentException(nameof(inputBase), nameof(outputBase));

It's unclear as to why you first check for <= 1 and then <= 0. It makes sense to not allow any base below 2, so the first two tests make sense.

There is no additional case that the third test will catch. If the third validation fails, then either the first or second validation has already failed and thus the third validation will never be run.

I think you need to revisit your test logic as the third test doesn't add anything of value. Additionally, I suggest commenting your validation logic to explain the intention, especially when you are using magic values like 1 or 0 and where the intention is not trivially readable.

---

The method names in AllYourBase can be improved.

• None of the private methods (see previous point) should have Rebase prepended to them. Their name should stand on its own.
• RebaseIsException implies a boolean return value, but it's void
• RebaseSolution is a confusing name. Methods take an imperative form (= command), and "to rebase a solution" isn't understandable. You presumably mean "the actual solution for the Rebase method", but that's not a good name.

My suggested renames are:

• RebaseIsException => ValidateInput
• RebaseIsDigitsEmptyOrZero => IsEmptyOrZero
• RebaseSolution => CalculateRebasedDigits

These methods are being using in Rebase, and their names were chosen to specifically highlight their responsibility as part of the Rebase superlogic.

// My suggestion

public static int[] Rebase(int inputBase, int[] digits, int outputBase)
{
    ValidateInput(inputBase, digits, outputBase);

    return IsEmptyOrZero(digits) 
               ? new int[] { 0 } 
               : CalculateRebasedDigits(inputBase, digits, outputBase);
}

---

Your unit tests are overall clear and readable.

What I'd change, though, is the naming. Currently, your test name describes the values you're arranging, but it's generally more meaningful to describe the expected behavior.

If I see a test called Both_bases_are_negative pass, that suggests to me that both bases being negative is a happy path. It obviously shouldn't be, so the test name should reflect that e.g. ThrowsException_WhenBothBasesAreNegative.

Similarly, Leading_zeros can be renamed to IgnoresLeadingZeroes. These are just two random examples of how you should introduce a description of the desired behavior.
As you may notice, I prefer to cut down on the amount of underscores in a test name, but that's personal preference.

---

You have a lot of unit tests that test the same thing with different values. xUnit has a better approach for this: theories. Very simply put, a theory is a fact with parameters; so you can run the same test for different values.

As an example, you can reduce these three facts:

[Fact]
public void Binary_to_multiple_decimal()
{
    var inputBase = 2;
    var digits = new[] { 1, 0, 1, 0, 1, 0 };
    var outputBase = 10;
    var expected = new[] { 4, 2 };
    Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
}

[Fact]
public void Decimal_to_binary()
{
    var inputBase = 10;
    var digits = new[] { 4, 2 };
    var outputBase = 2;
    var expected = new[] { 1, 0, 1, 0, 1, 0 };
    Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
}

[Fact]
public void Trinary_to_hexadecimal()
{
    var inputBase = 3;
    var digits = new[] { 1, 1, 2, 0 };
    var outputBase = 16;
    var expected = new[] { 2, 10 };
    Assert.Equal(expected, AllYourBase.Rebase(inputBase, digits, outputBase));
}

to a single theory:

[Theory]
[InlineData(2,  new[] { 1, 0, 1, 0, 1, 0 }, 10, new[] { 4, 2 })
[InlineData(3,  new[] { 1, 1, 2, 0 },       16, new[] { 2, 10 })
[InlineData(10, new[] { 4, 2 },             2,  new[] { 1, 0, 1, 0, 1, 0 })
public void Rebases_toOutputBase(int inputBase, int[] inputDigits, int outputBase, int[] expectedResult)
{
    var actual = AllYourBase.Rebase(inputBase, inputDigits, outputBase);

    Assert.Equal(expectedResult, actual);
}

There are ways to further improve this by using MemberData or ClassData attributes, which allows you to further abstract your test cases, but the general outset is the same: it prevents copy/pasting of structurally identical tests.

Answer 3

I like it. Coding is easy to read. In particular, here are the positives:

• Nice style and indentation
• Nice variable naming
• I like public and private declarations to each method.

The biggest area of contention is a matter of personal style, and that would be whether to always use braces after an if or foreach. The general consensus here at CR is that you should use them, but there is a significant number of developers who do not.

Getting pickier, RebaseIsDigitsEmptyOrZero and RebaseSolution should maybe be marked private. When you call them in Rebase, you have already validated all arguments. If it is public, I could call RebaseIsDigitsEmptyOrZero directly and either pass in a null digits or { -1, 1 }. The former would throw an exception but the latter would return true.

For a future challenge, if you dare, I was thinking of a class or struct that contains Digits and Base as properties that are co-joined in that class/struct. Then you could have a ConvertBase method accepting the new base as the argument, and it would return a new instance of that class/struct with the appropriate Digits and Base. Dunno. Still thinking about that one, but it might be worth exploring.

Nice job. I look forward to your future CR questions.

Answer 4

Thank you everyone for your help. This is what I refactored based on all the feedback.

using System;
using System.Collections.Generic;
using System.Linq;

public static class AllYourBase
{
    private static void ValidateInput(int inputBase, int[] digits, int outputBase)
    {
        if (inputBase <= 1)
            throw new ArgumentException(nameof(inputBase));

        if (digits.Any(e => e < 0 || e >= inputBase || digits is null))
            throw new ArgumentException(nameof(digits));

        if (outputBase <= 1)
            throw new ArgumentException(nameof(outputBase));
    }

    private static int[] CalculateRebasedDigits(int inputBase, int[] digits, int outputBase)
    {
        int number = 0;
        List<int> list = new List<int>();

        foreach (int i in digits)
            number = number * inputBase + i;

        do
        {
            list.Add(number % outputBase);
            number /= outputBase;
        } while (number != 0);

        list.Reverse();
        return list.ToArray();
    }

    public static int[] Rebase(int inputBase, int[] digits, int outputBase)
    {
        ValidateInput(inputBase, digits, outputBase);

        return CalculateRebasedDigits(inputBase, digits, outputBase);
    }
}