2
\$\begingroup\$

I developed a console app to generate and validate EAN-13 barcodes. I'm planning to develop a Windows Forms counterpart to render them as images using a barcode font.

I may test the console interface using Process.

Calculator.cs

using System;

namespace Ean13Calc
{
    public static class InternationalArticleNumber13Calculator
    {
        private static int Sz = 12;

        public static string InternationalArticleNumber13(int[] firstDigits = null)
        {
            int alt, digit, checkDigit, summedProduct = 0;
            Random randomDigits = new Random();
            bool isNull;
            if (firstDigits == null)
            {
                firstDigits = new int[Sz];
                isNull = true;
            }
            else
                isNull = false;
            for (int idx = 0; idx < Sz; idx++)
            {
                if (idx % 2 == 0)
                    alt = 1;
                else
                    alt = 3;
                if (isNull)
                {
                    digit = randomDigits.Next(10);
                    firstDigits[idx] = digit;
                }
                else
                    digit = firstDigits[idx];
                summedProduct += digit * alt;
            }
            checkDigit = 10 - summedProduct % 10;
            if (checkDigit == 10)
                checkDigit = 0;
            return string.Join("", firstDigits) + checkDigit.ToString();
        }

        public static string CalculateFromParsedNumber(string number)
        {
            int[] firstDigits = new int[Sz];
            int digit;
            if (number.Length != Sz | !long.TryParse(number, out long _))
                return null;
            for (int idx = 0; idx < Sz; idx++)
            {
                digit = int.Parse(number[idx].ToString());
                firstDigits[idx] = digit;
            }
            return InternationalArticleNumber13(firstDigits);
        }

        public static bool Validate(string number)
        {
            if (number.Length != Sz + 1)
                return false;
            return number == CalculateFromParsedNumber(number.Remove(Sz));
        }
    }
}

Program.cs

using System;
using System.Windows.Forms;

namespace Ean13Calc
{
    class Program
    {
        [STAThread]
        static void Main(string[] args)
        {
            string result;
            if (args.Length == 2 && args[0] == "-v")
            {
                Console.WriteLine("Is valid: {0}", InternationalArticleNumber13Calculator.Validate(args[1]));
                return;
            }
            if (args.Length == 1)
                result = InternationalArticleNumber13Calculator.CalculateFromParsedNumber(args[0]);
            else
                if (args.Length == 0)
                result = InternationalArticleNumber13Calculator.InternationalArticleNumber13();
            else
                return;
            if (result == null)
                Console.WriteLine("Error: expects 12 digits");
            else
            {
                Clipboard.Clear();
                Clipboard.SetText(result);
                Console.WriteLine("Barcode (in clipboard): {0}", result);
            }
        }
    }
}

TestCase.cs

using Microsoft.VisualStudio.TestTools.UnitTesting;

using Ean13Calc;

namespace Ean13Test
{
    [TestClass]
    public class TestCase
    {
        [TestMethod]
        public void TestWithDigits()
        {
            string firstDigits = "978030640615";
            string result = InternationalArticleNumber13Calculator.CalculateFromParsedNumber(firstDigits);
            Assert.AreEqual(firstDigits + "7", result);
            Assert.AreEqual(result.Length, 13);
        }

        [TestMethod]
        public void TestLength()
        {
            string result;
            for (int i = 0; i < 1000; i++)
            {
                result = InternationalArticleNumber13Calculator.InternationalArticleNumber13();
                Assert.AreEqual(result.Length, 13);
                Assert.IsTrue(InternationalArticleNumber13Calculator.Validate(result));
            }
        }

        [TestMethod]
        public void TestWrongInput()
        {
            string result = InternationalArticleNumber13Calculator.CalculateFromParsedNumber("0");
            Assert.IsNull(result);
        }
    }
}
\$\endgroup\$
0
\$\begingroup\$

I’m just going to go line by line through part of your calculator class.

namespace Ean13Calc
{
    public static class InternationalArticleNumber13Calculator
    {

Good name for the class. Very descriptive. Normally I wouldn’t advocate for abbreviating, but I actually think it’s best to abbreviate this one. Having worked in retail, everyone in the domain knows what an EAN is. Nearly no one knows what it stands for. Use the domain term and abbreviation for this one.

(To be clear, I would normally tell you the opposite. Abbreviating things shouldn’t be the norm.)

   private static int Sz = 12;

Like here. There’s no reason to abbreviate this. Use Size, or better EANLength. Also, this should be a constant so you can’t accidentally assign a new value to it.

   public static string InternationalArticleNumber13(int[] firstDigits = null)
    {

Why are you setting the default value to null? An array is already null by default. If you want a version of this method that doesn’t take any methods, use an overload with no parameters. It’s much less error prone than intentionally introducing nulls into your code.

       int alt, digit, checkDigit, summedProduct = 0;

I’m relatively certain this only works by accident. The default value of an int is zero, so I’m actually uncertain of the actual behavior here (and I’ve been using C# a long time).

Consider the alternative code.

int alt, digit, checkDigit, summedProduct = 1;

Are alt, digit, and checkDigit zero or one?

Only declare one variable per line and we don’t ever have to think about it.

        bool isNull;
        if (firstDigits == null)
        {
            firstDigits = new int[Sz];
            isNull = true;
        }
        else
            isNull = false;
        for (int idx = 0; idx < Sz; idx++)

Use braces around that else, or at least add a newline before the next statement.

—-

There’s more, but that’s probably a good start. I’d try to fix those issues (everywhere, some of them show up in multiple places) to make it more readable then come back for a second review.

\$\endgroup\$

Your Answer

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