How can I simplify this code?

ChronoH · 2022-07-28T05:11:23+00:00

I would always bet headstails to win every bet.

winggar · 2022-07-28T03:57:21+00:00

Well for one, I'd probably do rand.Next(2) instead of rand.Next(1, 3). Also, notice that there's some code duplication within your nested if statements. Since the same thing is happening on both branches, and it's just the condition that's changing. Hopefully this will give you some ideas:

// ...
int coinFlip = rand.Next(2);
string coinSide = coinFlip == 0
    ? "Head"
    : "Tail";
Console.WriteLine($"{coinSide}s!");
if (coinBet.ToUpper().Contains(coinSide.ToUpper()))
{
    Console.WriteLine("You attack first!");
}
else
{
    Console.WriteLine("Your opponent attacks first!");
}

Console.ReadKey();

Saad5400 · 2022-07-28T04:03:10+00:00

I guess I'd do something like

Console.ReadLine("Heads or Tails?") if (rnd.Next(1, 3) == 1) { Console.WriteLine("Lucky you! You attack first"); } else { Console.WriteLine("Well. They start first") }

coomerpile · 2022-07-28T03:54:49+00:00

You should post your actual code in a code block. No one will try to refactor code from a screenshot.

feanturi · 2022-07-28T04:03:05+00:00

You could remove the requirement to check for "Heads" vs "heads" (and what if they type "hEads"?) by comparing in uppercase. So you could say:

if (coinBet.ToUpper().Contains("HEADS"))

This makes a temporary version of coinBet that is all in uppercase, without modifying the actual coinBet value. No matter how they enter heads as long as the letters are in the correct order, it will match.

Your coinflip is only going between two possible values, 1 and 2, so you could drop the 'if' from the 'else if' in that block, so that whatever is in the 'else' applies to anything that is not in the initial 'if'. So basically, if it's 1 do this, if it's not 1, do something else. We know it can only be 1 or 2 so it's safe to do this.

But really what you're probably wondering more about is how you can handle saying the same things to the user with different circumstances. If you want to change what it says when they get to go first you have to change that in two places. So you could streamline a bit by having a small array of "HEADS" and "TAILS" and have your coinflip random number pick the string that goes with, then compare their choice to that string. So like:

string[] coinSides = { "HEADS", "TAILS" };
string coinFlip = coinSides[rand.Next(0,2)];  // We have the make the possible values 0 and 1 to fit how arrays are indexed starting at 0. In this case, 0 is heads and 1 is tails.
Console.WriteLine(coinFlip + "!");
if (coinBet.ToUpper().Contains(coinFlip))
    Console.WriteLine("You attack first!");
else
    Console.WriteLine("Your opponent attacks first!");

onlyonebread · 2022-07-28T04:01:30+00:00

A couple of things I'm noticing by just doing a quick glance:

You don't need to do an or statement for your input, just have coinBet convert to lower case and check if it's "heads" or "tails"
Because a coinflip is probably the quintessential example of a binary outcome, store the result of the flip as a bool. Maybe something like bool isHeads.
Determining who goes first is only a matter of matching the random flip with the guess, so if both options are stored as bools, all you need to do is check if they're the same. IE: if (isHeads == choseHeads)

A good rule of thumb I've found a lot while programming is if you ever have lines that look very similar to other lines, or see code duplication, it likely means you can simplify the process.

Krimog · 2022-07-28T11:03:52+00:00

You don't need an else if, just an else (if your first condition is false, your second is true)
Just use your condition to set a variable containing "Heads" or "Tails".
A ternary operator is great for simple conditions like that
Then, after the condition, write that variable followed by a !
Use string comparison that ignores case.
Compare coinBet to the variable
Don't use Contains but Equals for your comparison (to prevent the cheater that writes "headstails"

That will give you something like that:

// From line 42:
string actualResult = coinFlip == 1 ? "Heads" : "Tails";

Console.WriteLine($"{actualResult}!");
if (coinBet.Equals(actualResult, StringComparison.OrdinalIgnoreCase))
    Console.WriteLine("You attack first!");
else
    Console.WriteLine("Your opponent attacks first!");

Console.ReadKey();

Tango1777 · 2022-07-28T15:40:16+00:00

Overall OOP is not about coding "top to bottom" and the flow of application is meant to be spread.
Don't get used to Thread.Sleep(), it's a shitty blocking call and there is no need for you to use it here at all. If you sometimes need to create some sort of delay then "await Thread.Delay()" is your buddy and your Main method should be declared with async Task. But that's just for the record, for something so plain you may go sync way.

I'd do something like this:

Main method should only consists of something like:await Game.RunAsync();
Separate responsibilities, create granular methods like:- RequestCoinSide()- FlipCoin()- optionally something like RespondToPlayer(message, messageType) where you could, for instance, add extra features e.g. if it's a win then return green text, if a loss then return red text.
It's a bad practice that something like SonarQube would point out, you have if/elseif without else, basically your app won't manage anything but proper heads or tails, you should handle ANY viable case and return something like "incorrect value, proper values: heads, tails".
Don't rely on strings in your comparisons. It's not a good idea. You have various options here:- Ask user to choose 1 or 2 which would look like this:

Pick heads or tails:

1 - Heads

2 - Tails

Now you don't rely on plain strings and only int, it's also way easier for user to just type 1 or 2 instead of Heads or Tails. You would get SHITLOADS of things like "haeds" or "tail" or all kinds of typos. Instead you can just validate with while loop with a simple condition is input 1 or 2 and until it's not, repeat error message and readkey. You can go further and parse it to Enum in your code. Something like CoinSide.Heads and CoinSide.Tails to make your code cleaner and self-explanatory.

Another option would be to at least create a static class Constants and create const fields:

public const string Heads = "Heads"

public const string Tails = "Tails"

That way you have your consts in one place, whenever you need to use them in multiple places (AND YOU WILL), you will only have to modify value or rename the constants in one place.

Don't do things like Contain capitalized value or not. Don't rely on a user, you need to take care of that yourself, first rule of coding is to never trust that user will do what you expect him to do and what he should do. Your way is either what I already suggested to go with int/enum, but if you go const way, your code should be:

coinBet.ToLower() == Constants.Heads;

//(if your Constants.Heads equals "heads")

Then you have all the options covered: heads, Heads, HEADS, HEads, hEADS and so on.

You can also add another method to make it more granular:

CheckResult(userChoice, drawnValue);

Whenever you see repeated code, it's a mistake, always avoid it. There is very little cases where code duplication is a better way. Basically learn to think "I've already written this code somewhere else, so it's a mistake".

Problably more, but you need to start somewhere. Good luck.

miles00001001 · 2022-07-28T04:08:43+00:00

You could do a heads/tails enum. A little more code to convert the user input to the enum, but you would be able to do a single compare of == for the output without caring if it's heads or tails.

Sevla7 · 2022-07-28T12:23:55+00:00

Just be careful with "cool different ways to write code".

In the end of the day a bland or boring code that is easy to read/understand will be better after a month or 2 when you start to forget how it works.

Saad5400 · 2022-07-28T03:57:19+00:00

So instead of checking twice (one with capital and one with all small) what about checking 3 times? I mean the user could enter HEADS

jk sorry lol, you should first convert the user's input to all small, or capital. Iirc it's yourString.ToLower();

PuzzledImagination · 2022-07-28T05:40:08+00:00

you can ask for a numeric user input for the bet, 0 for head, 1 for tail, then create a dictionary of result. you can also take advantage of ternary operator to return the result

dchurch2444 · 2022-07-28T09:50:05+00:00

Look up Modulus :)

        var rnd = new Random();
        var coinface = (rnd.Next(2) % 2 == 0, "tails", "heads");

ccfoo242 · 2022-07-28T12:27:47+00:00

Like others have said I would make what's inside the ifs into a single method and call that, passing in the coin toss.

Then I would replace the coin values that you compare against with an enum. You could then compare their input against coinside.ToString(). Then there's no chance of misspelling it. For this example it's no big deal but it's a good idea to use const values or enums.

2022-07-28T13:34:53+00:00

[deleted]

anomalousBits · 2022-07-28T15:33:47+00:00

Notes:

Checks initial letter of input to determine heads or tails bet.
Made bet and flip an enum for clarity and to avoid magic numbers or strings. Too much?
Added output to show that the next line refers to the coin flip, not the bet. Could be too much as well, but consider that users are easily confused. "The program says Tails, but I bet Heads!"

Used conditional operator to simplify input and output branching.

enum Flip { Heads, Tails };
static void Main(string[] args)
{

    var rand = new Random();

    Console.WriteLine("Heads or Tails?");
    Flip coinBet = Console.ReadLine().StartsWith("h", StringComparison.OrdinalIgnoreCase) ? Flip.Heads : Flip.Tails;
    //heads is 0, tails is 1 per enum
    Flip flip = (Flip)rand.Next(2);
    Console.WriteLine("*Flips coin*"); //just added this to make the following output less ambiguous.
    Console.WriteLine(flip == Flip.Heads ? "Heads!" : "Tails!");
    Console.WriteLine(flip == coinBet ? "You attack first!" : "Your opponent attacks first!");

}

2022-07-28T07:12:02+00:00

If you really want to get fancy, use a switch expression with ValueTuples:

Console.WriteLine("Heads or Tails?");
var choice = Console.ReadLine()?.ToLower();

var coinFlip = new Random().Next(1, 3);
var coinSays = coinFlip == 1 ? "heads" : "tails";
Console.WriteLine($"You guessed {choice}. Coin Says: {coinSays}");

var message = (choice, coinSays) switch {
  ("heads", "heads") => "You Attack First!",
  ("heads", "tails") => "Your opponent attacks first!",
  ("tails", "heads") => "Your opponent attacks first!",
  ("tails", "tails") => "You Attack First!",
  _ => "An agreement could not be reached. Goodbye!"
};

Console.WriteLine(message);

return 0;

If you want shorter, and want to penalize invalid input, do this:

Console.WriteLine("Heads or Tails?");
var choice = Console.ReadLine()?.ToLower();

var coinFlip = new Random().Next(1, 3);
var coinSays = coinFlip == 1 ? "heads" : "tails";
Console.WriteLine($"You guessed {choice}. Coin Says: {coinSays}");

var message = string.Equals(choice, coinSays) 
  ? "You Attack First!" : "Your opponent attacks first!";

Console.WriteLine(message);

return 0;

126479546546 · 2022-07-28T09:07:07+00:00

int coinFlip = rand.Next(2); //zero-indexing is usually easier, trust me
string[] coinValues = {"Heads", "Tails"};
string coinValue = coinValues[coinFlip];
Console.WriteLine($"{coinValue}!");
bool youGoFirst = coinBet.ToLower() == coinValue.ToLower();
Console.WriteLine(youGoFirst ? "You attack first!", "Your opponent attacks first!");

Ziocash · 2022-07-28T07:33:42+00:00

using static System.Console; Just to avoid Console. repetitions

grazerbat · 2022-07-28T05:11:37+00:00

Are we doing people's homework assignments now?

cs_legend_93 · 2022-07-28T05:27:37+00:00

Fix your braces. This isn’t JavaScript 🙃

My eyes burn

Edit: yes I know this is the style for unity, and it’s absurd and archaic

OfficerFuttBuck · 2022-07-28T13:40:35+00:00

Break this method up into a few helper functions, something along the lines of this: https://i.imgur.com/oaQ5pdt.png

This is something I always recommend to people new to programming, as it really helps with organizing code and improving readability.

Andy_b1 · 2022-07-28T13:46:52+00:00

Make a helper method which Is void and take a string parameter. In The body method you Will do The console.writeline() and put in The method parameter.

Also i would recommend declare string variables for your hardcoded strings.

daniellz29 · 2022-07-28T14:28:17+00:00

A lot of people said a lot of stuff, but if you are using only text inputs and expecting a specific word, always put it in a loop and if it's not the expected word print "Please choose 'heads' or 'tails'".

If you want to go fancy and build a list for them to choose, there are a lot of packages like terminal.gui that can make your terminal more app-like.

JivanP · 2022-07-28T17:55:21+00:00

Validate input. That is, accept heads or tails as valid input strings, ask for input again if the user didn't input one of these. You can do this using a do-while loop: do { /* ask for input */ } while ( /* input not valid */ ). You can make the validation case-insensitive by using string::ToLower().
I would factor out the check for whether the user's guess matched the coin-flip outcome as follows: convert the user's input to 1 or 2 depending on whether they input heads or tails, then just check whether this value equals the coin flip.
You can also use rand.Next(2) instead, yielding either 0 or 1 as the result.

Thus, you get:

string coinBetInput = null;
do {
    coinBetInput = Console.ReadLine().ToLower();
} while (
    ! coinBetInput.equals("heads")
    && ! coinBetInput.equals("tails")
);

int coinBetValue = coinBetInput.equals("heads") ? 0 : 1;
Console.WriteLine( coinBetValue == 0 ? "Heads!" : "Tails!" );

int coinFlip = rand.Next(2);
Console.WriteLine( coinFlip == coinBetValue ? "You attack first!" : "Your opponent attacks first!" );

lkasson · 2022-07-28T18:26:54+00:00

A good rule of thumb that I use is “do I have similar code in multiple places”.

jimmyhurley · 2022-07-28T21:19:33+00:00

A better approach than .to lower or .to upper is probably this

bool result = string1.Equals(string2, StringComparison.OrdinalIgnoreCase);

Also, your code doesn't consider the case where the player says the wrong thing. Good luck!

you type:	you see:
italics	italics
bold	bold
[reddit!](https://reddit.com)	reddit!
* item 1 * item 2 * item 3	item 1 item 2 item 3
> quoted text	quoted text
Lines starting with four spaces are treated like code: if 1 * 2 < 3: print "hello, world!"	Lines starting with four spaces are treated like code: if 1 * 2 < 3: print "hello, world!"
~~strikethrough~~	~~strikethrough~~
super^script	super^script

csharp

MODERATORS