Show a diamond shape with numbers

Question

After seeing this post on StackOverflow, I thought I'd try it out. What I have now is a diamond shape with numbers like this:

    1
   212
  32123
 4321234
543212345
 4321234
  32123
   212
    1

My code looks like this:

int i, j, k, n = 6, l = 3, o = 2;
for (i = 1; i <= 5; i++)
{
    for (k = n; k >= 1; k--)
    {
        Console.Write(" ");
    }
    n--;
    for (j = i; j >= 1; j--)
    {
        Console.Write(j);
    }
    o = 2;
    for (j = 1; j < i; j++)
    {
        Console.Write(o);
        o++;
    }
    Console.WriteLine();
}
for (i = 4; i >= 1; i--)
{
    for (k = l; k >= 1; k--)
    {
        Console.Write(" ");
    }
    l++;
    for (j = i; j >= 1; j--)
    {
        Console.Write(j);
    }
    o = 2;
    for (j = i; j > 1; j--)
    {
        Console.Write(o);
        o++;
    }
    Console.WriteLine();
}
Console.Read();

I don't think this is a nice solution for this task. Is there any nicer and shorter way to solve this without using that many loops?

Answer 1

Code issues that decrease readability and maintainability

• Plenty of loops.
• Non-meaningful variable names: n, l, o.

---

We can declare the following helper methods:

private static string CreateDigitsString(int length)
{
    char[] buffer = new char[length];
    for (int i = 0; i < buffer.Length; i++)
    {
        buffer[i] = (char)('1' + Math.Abs(buffer.Length / 2 - i));
    }
    return new string(buffer);
}

private static string CreateLine(int length, int spaces)
{
    return new string(' ', spaces) + CreateDigitsString(length - 2 * spaces) + new string(' ', spaces);
}

Then use it like:

const int N = 5;
const int TotalRows = N * 2 - 1;

for (int row = 0; row < TotalRows; row++)
{
    Console.WriteLine(CreateLine(TotalRows, Math.Abs(N - row - 1)));
}

---

Another approach
In the 2-dimensional loop (i, j) we can calculate the distance between the current cell and the center of the diamond as follows:

int rowDistance = Math.Abs(row - N + 1);
int colDistance = Math.Abs(col - N + 1);

Let's output the colDistance for cells that are within rowDistance + colDistance <= N - 1, and space - otherwise.

const int N = 5;
const int TotalSize = N * 2 - 1;

for (int row = 0; row < TotalSize; row++)
{
    for (int col = 0; col < TotalSize; col++)
    {
        int rowDistance = Math.Abs(row - N + 1);
        int colDistance = Math.Abs(col - N + 1);
        Console.Write(rowDistance + colDistance <= N - 1 ? (char)('1' + colDistance) : ' ');
    }
    Console.WriteLine();
}

Answer 2

Any time you see a loop, ask yourself "can I move the loop into a helper method?"

Let's make some very simple helper methods that have the loops:

Here's a method that takes a sequence of T and a new T, and returns you a sequence with the new T appended to the end:

public static IEnumerable<T> Append<T>(this IEnumerable<T> items, T new_item)
{
    foreach(T item in items)
        yield return item;
    yield return new_item;
}

Pretty easy.

Here's a method that counts from 1 to n:

static IEnumerable<int> CountUp(int n)
{
    for(int i = 1; i <= n; ++i) 
        yield return i;
}

And from n to 1:

static IEnumerable<int> CountDown(int n)
{
    for(int i = n; i >= 1; --i) 
        yield return i;
}

And gives you N of a thing:

static IEnumerable<T> Repeat<T>(T item, int n)
{
    for (int i = 0; i < n; ++i)
        yield return item;
}

I hope you agree these methods are all very simple. But now that we have them we can construct a solution to your problem that uses no loops:

static IEnumerable<string> Diamond(int n)
{
    return 
        from i in CountUp(n).Concat(CountDown(n - 1))
        from s in 
          Repeat(" ", n - i).
          Concat(CountUp(i).Select(x => x.ToString())).
          Concat(CountDown(i - 1).Select(x => x.ToString())).
          Append("\n")
        select s;
}

Notice how the code reads like the operations that we want it to perform. We want to count up from 1 to n and then down from n-1 to 1. For each of those things we want to repeat some number of spaces, then concatenate to that some numbers converted to strings, and then concatenate some more numbers converted to strings, and then a newline.

Put it all together: https://dotnetfiddle.net/BRegu7

By moving the loops into plainly-correct helper methods, you enable your code to read like what you want the result to be logically and not read like the steps taken through all the loops to get there. Emphasize the semantics of your code, not the mechanisms that make it work.

Answer 3

What's wrong?

Your code isn't easily extendable and there are a lot of repetitions. This means if you want to render numbers bigger then 9 you need to adjust multiple lines. Actually you need to rewrite the entire function. If you wanted to print the results to something else then the Console - you need to change everything agian.

You cannot write tests for it because all calculations go directly to the console and you can validate them only visually.

How to fix it?

In order to optimize code you need to think like a mechanical engineer and ask yourself what parts will you need in your machine so that it does what you want it to do. Why? Because when you build a machine you cannot build a part that does everything - like your method does.

But if you see it as a machine then you can easier extract all the specialized parts for each task. It'll help you to think in OO-way so that you can extract those modules and apply SOLID principles.

Let's start...

• The diamond machine should oscillate between certain numbers, this means we need an oscillator Part 1
• The diamond machine should generate lines of numbers, this means we need a diamond generator Part 2
• The diamond machine should display the result, this means we need a display or rather a renderer in this case Part 3

Here's an example how such parts could look like.

Part 1 - Oscillator

This part is responsible for creating collections of numbers oscillating between the given number.

interface IOscillator
{
    IEnumerable<int> Oscilate(int between);
}

class Oscillator : IOscillator
{ 
    public IEnumerable<int> Oscillate(int between)
    {
        var sign = 1;
        for (var i = between; i <= between; i = i - 1 * sign)
        {
            yield return i;
            if (i == 1) { sign = -1; }
        }
    }
}

Part 2 - Diamond generator

The diamond generator requires an oscillator to work. You specify one with dependency incjection by an abstraction layer IOscillator. This part can use the oscillator to generate lines for the diamond. It also validate the size which has to be an odd number.

class Diamond : IEnumerable<DiamondLine>
{
    private readonly int _size;
    private readonly IOscillator _oscillator;
    public Diamond(int size, IOscillator oscillator)
    {
        if (size % 2 == 0) { throw new ArgumentOutOfRangeException("Width must be odd"); }
        _size = size;
        _oscillator = oscillator;
    }

    private int Max => (_size + 1) / 2;

    private IEnumerable<DiamondLine> GenerateDiamond()
    {
        foreach (var number in _oscillator.Oscillate(Max))
        {
            var currentMax = Max - number + 1;
            var offset = Max - currentMax;
            yield return new DiamondLine(offset, _oscillator.Oscillate(currentMax));
        }
    }

    public IEnumerator<DiamondLine> GetEnumerator() => GenerateDiamond().GetEnumerator();
    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

Each diamond line has specific properties. Those are encapsulated by the DiamondLine. Our renderer will know what to do with it.

class DiamondLine
{
    public DiamondLine(int offset, IEnumerable<int> numbers) 
    {
        Offset = offset;
        Numbers = numbers;
    }
    public int Offset { get; }
    public IEnumerable<int> Numbers { get; }
}

Part 3 - Diamond Renderer

In the final part we will render each DiamondLine as a string.

class DiamondRenderer
{ 
    public IEnumerable<string> RenderDiamond(Diamond diamond)
    {
        foreach (var line in diamond)
        {
            yield return new string(' ', line.Offset) + string.Join(string.Empty, line.Numbers);
        }
    }
}

Usage

Now that you have so many parts, you can assemble them and print the lines to whatever you want.

var diamond = new Diamond(7, new Oscillator());
var diamondRenderer = new DiamondRenderer();
foreach (var line in diamondRenderer.RenderDiamond(diamond))
{
    Console.WriteLine(line);
}

and this is the result:

I don't use a monotype font in LINQPad so the numbers are not perfectly alligned.

You might ask why so complicated? Becasue we want to separate all concerns. You have a working oscillator, a generator and a renderer.

If you now decide to extend it for numbers bigger then 9 you will only need to adjust the renderer so it renders numbers prefixed with a 0 like 03.

Testability

Now each part can also be tested. Separately. You can write a better oscillator and test it or a renderer.

---

Possibilities

I've created a simple renderer but you could add an abstraction layer to it and use it via DI to implement the ToString method of the Diamond. You could then write various renderers that render simple strings or html or xml or whatever you want.

As long as you keep each functionality in a separate class you can easily extend it and make adjustments to any of them without affecting any other working parts.

---

Desclimer: I'm not a matematician so there might be a cooler way to write the oscillator.

Answer 4

Overall your code does what you intend and that's a good start: to print a diamond with numbers.

The first improvement could be to handle an "arbitrary" sized diamond:

static void CreateDiamond(int size)
{
  if (size > 9)
  {
    Console.WriteLine("Size must be between 1 and 9 inclusive");
    return;
  }

  int n = size;
  int l = 0;
  int i, j, k, o;
  for (i = 1; i <= size; i++)
  {
    for (k = n; k >= 1; k--)
    {
      Console.Write(" ");
    }
    n--;
    for (j = i; j >= 1; j--)
    {
      Console.Write(j);
    }
    o = 2;
    for (j = 1; j < i; j++)
    {
      Console.Write(o);
      o++;
    }
    Console.WriteLine();
  }

  l = 2;
  for (i = size - 1; i >= 1; i--)
  {
    for (k = 0; k < l; k++)
    {
      Console.Write(" ");
    }
    l++;
    for (j = i; j >= 1; j--)
    {
      Console.Write(j);
    }
    o = 2;
    for (j = i; j > 1; j--)
    {
      Console.Write(o);
      o++;
    }
    Console.WriteLine();
  }
}

Next could be to refactor the variables which it self makes it much more readable for everyone:

static void CreateDiamond1(int size)
{
  if (size < 1 || size > 9)
  {
    Console.WriteLine("Size must be between 1 and 9 inclusive");
    return;
  }

  int indent = size;
  int rightSideIndex;

  // The upper part of the diamond
  for (int i = 1; i <= size; i++) 
  {
    for (int j = indent; j >= 1; j--)
    {
      Console.Write(" ");
    }
    indent--;
    for (int j = i; j >= 1; j--)
    {
      Console.Write(j);
    }
    rightSideIndex = 2;
    for (int j = 1; j < i; j++)
    {
      Console.Write(rightSideIndex);
      rightSideIndex++;
    }
    Console.WriteLine();
  }

  // The lower part of the diamond
  indent = 2;
  for (int i = size - 1; i >= 1; i--)
  {
    for (int j = 0; j < indent; j++)
    {
      Console.Write(" ");
    }
    indent++;
    for (int j = i; j >= 1; j--)
    {
      Console.Write(j);
    }
    rightSideIndex = 2;
    for (int j = i; j > 1; j--)
    {
      Console.Write(rightSideIndex);
      rightSideIndex++;
    }
    Console.WriteLine();
  }
}

Then we can make some improvements of the algorithm:

static void CreateDiamond2(int size)
{
  if (size < 1 || size > 9)
  {
    Console.WriteLine("Size must be between 1 and 9 inclusive");
    return;
  }

  // Use linq to create a sequence of the size of each line in the diamond
  // for instance the third line is: 12321
  var lineSizes = Enumerable.Range(1, size);
  lineSizes = lineSizes.Concat(lineSizes.Reverse().Skip(1));

  foreach (var lineSize in lineSizes)
  {
    // Indent at left
    for (int i = 0; i < size - lineSize; i++)
    {
      Console.Write(" ");
    }

    // Write the digits
    int offset = -1;
    for (int i = lineSize; i <= lineSize; i += offset)
    {
      Console.Write(i);
      if (i == 1)
        offset = -offset;
    }

    Console.WriteLine();
  }
}

Second improvement is to (almost) completely use linq to generate the diamond:

// Mirrors the sequence from 1 to size to 1
static IEnumerable<int> AssembleLine(IEnumerable<int> sequence, int size)
{
  return sequence.Concat(sequence.Reverse().Skip(1));
}

static void CreateDiamond3(int size)
{
  if (size < 1 || size > 9)
  {
    Console.WriteLine("Size must be between 1 and 9 inclusive");
    return;
  }

  // Use linq to create a sequence of the size of each line in the diamond
  var lineSizes = AssembleLine(Enumerable.Range(1, size), size);

  foreach (var lineSize in lineSizes)
  {
    var digits = AssembleLine(lineSizes.Take(lineSize).Reverse(), lineSize);

    Console.WriteLine($"{new string(' ', size - lineSize)}{string.Join("", digits)}");
  }
}

Answer 5

If you consider the top to be a mirror of the bottom and the right a mirror of the left, you can do it in one loop, but it still takes two loops to print it.

I get this is a departure from the OP but getting rid of loops is going to be a departure.

public static void diamod(int size)
{   // size is the max number 
    int dim = 2 * size - 1;
    int?[,] dia = new int?[dim, dim];
    for (int i = 0; i < size; i++)
    {
        for (int j = 0; j <= i; j++)
        {
            // middle out 
            dia[i, size - 1 - j] = j + 1;
            dia[i, size - 1 + j] = j + 1;

            // mirror the bottom
            dia[dim - 1 - i, size - 1 - j] = j + 1;
            dia[dim - 1 - i, size - 1 + j] = j + 1;
        }
    }
    for (int i = 0; i < dim; i++)
    {
        for (int k = 0; k < dim; k++)
        {
            Debug.Write(dia[i, k] == null ? " " : dia[i, k].ToString());
        }
        Debug.WriteLine("");
    }
}