I am learning C# like 1 or 2 months and I wanted to know if there is any possibility to shorten this code.
It is simple. It only changes color of the console text with while
loop if you enter wrong input. When you enter it correctly it changes the color + asks you if you want to continue.
It looks weird for me when I have for each color duplicity of the code written before (in if and switch statements) But I have no idea if there is any way to make it cleaner and shorter, but I guess there is. Also I would like to know where is worth to use arrays and where normal strings and integers + if there is any reasonable difference. Also I find it very hard for orientation when I have a lot of strings or ints inside array, but easy to maintain. Thanks for answers
static void Main(string[] args)
{
string[] Arrays = new string[3]{
"Hello!", "Enter a color\n1 = blue\n2 = red\n3 = Magenta", "Render numbers"
Console.WriteLine(Arrays[0]);
bool Continue = true;
while(Continue) {
Console.WriteLine(Arrays[1]);
string[] color = new string[4]{
"Blue", "Red", "Magenta", "Render numbers : "
};
char EnterColor = Console.ReadKey().KeyChar;
Console.WriteLine();
switch (EnterColor) {
case '1':
Console.ForegroundColor = ConsoleColor.Blue;
Console.WriteLine("Do you want to continue? y/n");
string NextTask = Console.ReadLine();
if (NextTask == "y") {
// set loop to repeat
Continue = true;
}
else {
Continue = false;
}
break;
case '2':
Console.ForegroundColor = ConsoleColor.Red;
Continue = false;
Console.WriteLine("Do you want to continue? y/n");
string NextTaskRed = Console.ReadLine();
if (NextTaskRed == "y") {
Continue = true;
}
else {
Continue = false;
}
break;
case '3':
Console.ForegroundColor = ConsoleColor.Magenta;
Continue = false;
Console.WriteLine("Do you want to continue? y/n");
string NextTasMagenta = Console.ReadLine();
if (NextTasMagenta == "y") {
// set loop to repeat
Continue = true;
}
else {
Continue = false;
}
break;
// there will be render nums option in next case using for loop
}
}
}
-
2\$\begingroup\$ Please paste all code, not just the middle of a method. \$\endgroup\$dfhwze– dfhwze2019年08月09日 19:30:16 +00:00Commented Aug 9, 2019 at 19:30
-
\$\begingroup\$ Alright i pasted whole code except usings. \$\endgroup\$xBeast12– xBeast122019年08月09日 19:40:32 +00:00Commented Aug 9, 2019 at 19:40
-
1\$\begingroup\$ We also need to know what your application is doing. Please discribe its purpose and how it works etc. \$\endgroup\$t3chb0t– t3chb0t2019年08月09日 19:55:49 +00:00Commented Aug 9, 2019 at 19:55
-
2\$\begingroup\$ It is simple it only changes color of the console text with while loop if you enter wrong input. When you enter it correctly it changes the color + asks you if you want to continue. \$\endgroup\$xBeast12– xBeast122019年08月09日 20:03:21 +00:00Commented Aug 9, 2019 at 20:03
1 Answer 1
Review
Whitespace
Use whitespace between the method name and opening parenthesis: Main (string[] args)
instead of Main(string[] args)
. The same convention applies to statements such as while: while (Continue)
instead of while(Continue)
. And also to curly braces: new string[4] {
instead of new string[4]{
.
New Line
Prefer using the new line of the system over a fixed format: use Environment.NewLine
instead of \n
.
Comments
Avoid useless comments such as //strings
. We all know these are string instances: "Hello!", "Enter a color\n1 = blue\n2 = red\n3 = Magenta", "Render numbers"
. Don't comment out code that you don't use, remove it entirely: //string Nums = "";
.
Naming conventions
Use pascalCase for variable names: string[] arrays
instead of string[] Arrays
. But what does arrays mean? Use a meaningful instead: string[] promptMessages
for instance.
Using an array or not
There is no reason to wrap the messages in an array. Console.WriteLine(Arrays[1]);
is a disaster for readability. Console.WriteLine("Enter a color ..");
is much cleaner. On the other hand, string[] color
could have been declared as ConsoleColor[] colors
. This way, you could have avoided all that repetitive code. You should try to write DRY code.
var colors = new ConsoleColor[] {
ConsoleColor.Blue,
ConsoleColor.Red,
ConsoleColor.Magenta
};
// NOTE: this is a trivial parsing, you should perform some validation..
// 0 for Blue, 1 for Red, 2 for Magenta
char userColor = int.Parse(Console.ReadKey().KeyChar.ToString());
Console.ForegroundColor = colors[userColor];
Console.WriteLine("Do you want to continue? y/n");
var nextTask = Console.ReadKey().KeyChar;
Continue = nextTask == 'y';
Console input
Don't read an entire line when you only need a single character: string NextTask = Console.ReadLine(); if (NextTask == "y") { ..
. You can read a single character: string NextTask = Console.ReadKey().KeyChar;
. Unlike ReadLine
, ReadKey
does not wait for Enter key to be pressed, but immediately returns the pressed key.
-
1\$\begingroup\$ You should add that
Console.ReadKey()
doesn't wait for the user to press enter. It returns on any key press. \$\endgroup\$Xiaoy312– Xiaoy3122019年08月09日 20:45:14 +00:00Commented Aug 9, 2019 at 20:45 -
\$\begingroup\$ Thanks a lot ! Yes it makes sense to me. The main method was generated by visual studio tho. But i still didn't get where is worth to use string arrays. \$\endgroup\$xBeast12– xBeast122019年08月09日 20:48:01 +00:00Commented Aug 9, 2019 at 20:48
-
\$\begingroup\$ @Xiaoy312 Good point, added. \$\endgroup\$dfhwze– dfhwze2019年08月09日 20:48:46 +00:00Commented Aug 9, 2019 at 20:48
-
\$\begingroup\$ You understand the part about ConsoleColor[] colors to avoid redundant code? Was I clear enough? \$\endgroup\$dfhwze– dfhwze2019年08月09日 20:50:22 +00:00Commented Aug 9, 2019 at 20:50
-
1\$\begingroup\$ Oh thanks a lot this is exactly what I was looking for. \$\endgroup\$xBeast12– xBeast122019年08月09日 20:57:51 +00:00Commented Aug 9, 2019 at 20:57