0
\$\begingroup\$

I created a method to print a playing field in the console, but I think there are much better ways to do this than mine.

public void DrawPlayingField()
    {
        
        for (int i = 5; i < 120; i++)
        {
            Console.SetCursorPosition(i, 1);
            Console.WriteLine('I');
        }
        for (int i = 1; i < 35; i++)
        {
            Console.SetCursorPosition(5, i);
            Console.WriteLine("II");
        }
        for (int i = 5; i  < 120; i++)
        {
            Console.SetCursorPosition(i, 34);
            Console.WriteLine('I');
        }
        for (int i = 1; i < 35; i++)
        {
            Console.SetCursorPosition(119, i);
            Console.WriteLine("II");
        }
    }

The output looks like it is supposed to look: picture

Are there any better solutions for my problem?

\$\endgroup\$
2
  • 3
    \$\begingroup\$ It would be much better if you provided the entire game for review and possibly re titled the question to be something like C# snake text game. \$\endgroup\$
    – pacmaninbw
    Commented Aug 31, 2022 at 19:00
  • 2
    \$\begingroup\$ Now imagine that someone says: I want the rectangle to be 5 places smaller on either side. How many changes do you need to make? Hint: if the number of changes is bigger than two, your code is bad. \$\endgroup\$
    – BCdotWEB
    Commented Sep 1, 2022 at 6:19

2 Answers 2

4
\$\begingroup\$

Lets do some refactoring exercise together

Reduce the number of loops

If you look at your for loops' iteration definitions

for (int i = 5; i < 120; i++)
for (int i = 1; i < 35; i++)
for (int i = 5; i < 120; i++)
for (int i = 1; i < 35; i++)

then you can see that you have only two different versions. So, you can combine those for loops which has the same iteration definition

for (int i = 5; i < 120; i++)
{
    Console.SetCursorPosition(i, 1);
    Console.WriteLine('I');
    Console.SetCursorPosition(i, 34);
    Console.WriteLine('I');
}

for (int i = 1; i < 35; i++)
{
    Console.SetCursorPosition(5, i);
    Console.WriteLine("II");
    Console.SetCursorPosition(119, i);
    Console.WriteLine("II");
}

Simplify horizontal lines logic

Your first iteration is responsible to write 115 I characters at 1st and the 34th line on the console.

Instead of writing 230 I characters one by one (+ setting the cursor's position) you can write two 115 long strings:

Console.SetCursorPosition(5, 1);
Console.WriteLine(new string('I', 115));
Console.SetCursorPosition(5, 34);
Console.WriteLine(new string('I', 115));

Improving legibility by using constants

If you introduce some constants then it can highly improve the readability of your code by better expressing your intent

Constants

const int LeftMargin = 5, TopMargin = 1;
const int Height = 34, Width = 115;
const char HorizontalSymbol = 'I';
const string VerticalSymbol = "II";

Their usage

Console.SetCursorPosition(LeftMargin, TopMargin);
Console.WriteLine(new string(HorizontalSymbol, Width));
Console.SetCursorPosition(LeftMargin, Height);
Console.WriteLine(new string(HorizontalSymbol, Width));

for (int line = TopMargin; line <= Height; line++)
{
    Console.SetCursorPosition(LeftMargin, line);
    Console.WriteLine(VerticalSymbol);
    Console.SetCursorPosition(LeftMargin + Width -1, line); //-1 cuz VerticalSymbol's length
    Console.WriteLine(VerticalSymbol);
}
\$\endgroup\$
1
\$\begingroup\$

A quick review of your code is that it is very fragile due to all of your magic numbers. Peter Csala addresses this with he suggests using constants.

(Forgive me for code below. I am writing this from scratch in browser as I do not have VS available on this PC.)

I would offer an alternative that goes beyond replacing magic numbers with constants. I would suggest you are better off with a PlayingField class and now those never changing constants can become properties for playing fields of different sizes.

public class PlayingField
{
   public int LeftMargin { get; }
   public int TopMargin { get; }
   public int Height { get; }
   public int Width { get; }

   public void DrawBorder()
   {
       // Left as excercise
   }

}

Here where it can get fun. You can add more properties that can add some dazzle. What about a ForeColor or BackColor property? Or maybe make that BorderForeColor and BorderBackColor (I am assuming you may draw other things inside the border). Maybe you want an InteriorMargin property. The main thing is to think of a general PlayingField object that has these properties that can add a greater flexibility to what you can do.

Even your ASCII art characters could also become named properties. All of these properties means changing the size, colors, and look is easily done by creating a new instance of PlayingField.

If you are on Windows, I would urge you to run charmap.exe and look at the Consolas font, which is the default for Windows consoles. Scroll down and you will see all sorts of nice new characters to experiment with.

enter image description here

Peter has sound advice on reducing the number of iterations. However, if you are later thinking of writing things within your playing field borders, then you would need to write location by location rather than the more efficient write a complete string. So it really depends on where else you plan to take this on whether this bit of Peter's advice is applicable.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.