Public Group

# Coding 'Eloquence' - C#

This topic is 2775 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

So I'd like to start off by saying what I'm asking for is someone more experience than I am to see if my code is eloquent, I don't know how else to put it. I looked around the other forums and decided for the type of question this was the best place for it. Anyways, to the meat and potatoes:

I started earnestly learning C# a little under 5 hours ago from the time of this writing. Previously I had dabbled in web coding, C++, and scripting languages. I really never made it far past the "Hello World" stage. But I dove into this great resource that was perfect for me. I got to the method lesson about 2 hours ago, and the challenge presented to me took the remaining 2ish hours. The challenge is to turn a piece of code from solely inside 'static main()' and run it as much as possible outside of 'main' without any 'static' methods. Summary of code at bottom!!! And the two specific questions.

*Note*
This is supposed to be "game" between a monster and hero, but really just math with options.

This is the original code:
using System; using System.Collections.Generic; using System.Linq; using System.Text; namespace Tutorial1 { class Program { static void Main(string[] args) { int heroHitPoints, monsterHitPoints, attackDamage; string battlechoice; Random rand; Console.WriteLine("You are facing a monster"); //this is outside the loop so that it will only print once heroHitPoints = 30;// our variables are assigned ouside monsterHitPoints = 25;//so that each loop won't "heal" them do { rand = new Random(); Console.Write(@" ************************************************ Your hero has {0}hp and the monster has {1}hp ************************************************", heroHitPoints, monsterHitPoints); Console.Write(@" __________________________ Please Choose an action: (A)ttack (D)efend __________________________"); Console.WriteLine(); battlechoice = Console.ReadLine(); switch (battlechoice) { case "a": case "A"://this way a or A work attackDamage = rand.Next(3, 7);//get our damage monsterHitPoints -= attackDamage;//subtract the damage Console.WriteLine("The hero attacks!"); Console.WriteLine("The monster loses {0}hp", attackDamage); break; case "d": case "D": Console.WriteLine("The Hero Defends"); break; default://defaults always a good idea with user input Console.WriteLine("Sorry that choice was invalid and the monster takes a cheap shot"); break; } Console.WriteLine(); if (monsterHitPoints > 0)//if the monster is still alive { Console.WriteLine("The Monster Attacks!"); attackDamage = rand.Next(3, 7);//get a random number if (battlechoice == "d" || battlechoice == "D") { //this is so that defend has some sort of benefit attackDamage /= 2; } heroHitPoints -= attackDamage;//subtract the damage Console.WriteLine("The Hero loses {0}hp", attackDamage); } Console.WriteLine("Press Enter to Continue"); Console.ReadLine(); Console.Clear();//this clears the screen so that we don't have the //last turns info on it. } while (heroHitPoints > 0 && monsterHitPoints > 0); if (heroHitPoints > 0) { Console.WriteLine("You have defeated the monster"); } else { Console.WriteLine("The monster has defeated you"); } Console.ReadLine(); } } }

and with the object being non-static methods I came up with this:
using System; using System.Collections.Generic; using System.Linq; using System.Text; namespace ConsoleApplication1 { class Program { int BattleChoice(string choice)//Receives input after 'Action Menu' appears and returns action chosen { int attackChoice = 0; //0 is no choice picked - 1 is attack - 2 is defend - 3 is health potion - 4 is damage potion switch (choice) { case "a": case "A": attackChoice = 1; break; case "d": case "D": attackChoice = 2; break; case "h": case "H": attackChoice = 3; break; case "p": case "P": attackChoice = 4; break; default: attackChoice = 0; break; } return attackChoice; } // Formatting comment int MonsterHealth(int health, int heroAttack)//Is called to obtain monster health before hit, and return post damage health { health -= heroAttack; return health; } // Formatting comment int MonsterAttack()//Is called to obtain damage for the monster, no outside modifiers { int attack; Random rand; rand = new Random(); attack = rand.Next(5, 10); return attack; } // Formatting comment int HeroHealth(int heroHealth, int monsterAttack) //Is called to obtain hero health before hit, and return post damage health { heroHealth -= monsterAttack; return heroHealth; } // Formatting comment int HeroAttack(int bonus) //Is called to obtain the damage for the hero { int attackDamage; Random rand; rand = new Random(); attackDamage = rand.Next(10, 20); if (bonus > 0) { attackDamage *= 2; } return attackDamage; } // Formatting comment static void Main(string[] args) { Program program = new Program(); // So I can get working methods int heroHitPoints, monsterHitPoints, heroAtkDmg, monsterAtkDmg; //stat variables int battleReturn, bonus; //battle action variables string localChoice; //created to send input to BattleChoice() heroHitPoints = 100; monsterHitPoints = 75; heroAtkDmg = 0; monsterAtkDmg = 0; bonus = 0; Console.WriteLine("A scary monster has appeared!"); Console.WriteLine(); Console.ReadLine(); do { Console.Clear(); Console.WriteLine(@" ************************************************ Your hero has {0}hp and the monster has {1}hp ************************************************", heroHitPoints, monsterHitPoints); Console.Write(@" __________________________ Please Choose an action: (A)ttack (D)efend (H)ealth Potion Damage (P)otion __________________________"); Console.WriteLine(); localChoice = Console.ReadLine(); battleReturn = program.BattleChoice(localChoice); //Below creates the if statement for the action menu. Using a method with this setup proved to be way to confusing if (battleReturn == 0) { Console.WriteLine("The monster has taken a cheap shot at you because you did something random."); monsterAtkDmg = program.MonsterAttack(); heroHitPoints = program.HeroHealth(heroHitPoints, monsterAtkDmg); Console.WriteLine("You have taken {0} points of damage.", monsterAtkDmg); } else if (battleReturn == 1) { heroAtkDmg = program.HeroAttack(bonus); Console.WriteLine("You attack the monster for {0} points of damage.", heroAtkDmg); monsterHitPoints = program.MonsterHealth(monsterHitPoints, heroAtkDmg); if (monsterHitPoints > 0) { monsterAtkDmg = program.MonsterAttack(); Console.WriteLine("The monster attacks back for {0} points of damage.", monsterAtkDmg); heroHitPoints = program.HeroHealth(heroHitPoints, monsterAtkDmg); } } else if (battleReturn == 2) { Console.WriteLine("You bolster your defenses preparing for the oncoming onslaught!"); monsterAtkDmg = program.MonsterAttack(); monsterAtkDmg /= 2; Console.ReadLine(); Console.WriteLine("Your defence has reduced the monsters damage in half, you only took {0} damage.", monsterAtkDmg); heroHitPoints = program.HeroHealth(heroHitPoints, monsterAtkDmg); } else if (battleReturn == 3) { Console.WriteLine("You chug a health potion returning lost hit points."); heroHitPoints = 100; monsterAtkDmg = program.MonsterAttack(); Console.WriteLine("The monster attacks you for {0} damage.", monsterAtkDmg); heroHitPoints = program.HeroHealth(heroHitPoints, monsterAtkDmg); } else if (battleReturn == 4) { Console.WriteLine("You chug a damage potion doubling your damage done."); bonus++; monsterAtkDmg = program.MonsterAttack(); Console.WriteLine("The monster attacks you for {0} damage.", monsterAtkDmg); heroHitPoints = program.HeroHealth(heroHitPoints, monsterAtkDmg); } heroAtkDmg = 0; // Resetting attack values to insure no numbers errors are created. monsterAtkDmg = 0; Console.WriteLine("Press Enter to Continue"); Console.ReadLine(); } while (heroHitPoints > 0 && monsterHitPoints > 0); //Creates an endpoint if (heroHitPoints > 0) { Console.WriteLine("You have defeated the monster"); } else { Console.WriteLine("The monster has defeated you"); } Console.ReadLine(); //End of Main } } } 

I wanted to know if this was even remotely close to writing eloquent code, that looks nice and is purposeful. Also, the if statement in main that deals with the action menu actions, I attempted to write that into a non-static method, but quickly grew lost in the ever increasing list of variables and arguments that each method required. One of the methods I created there were 6 arguments required to get it working correctly, two of those arguments were functions which had 3 arguments each. Am I over complicating that part, or does the rest of my code just not support those types of actions?

Summary of code:
Original:
A do/while loop inside of static main that chooses between two actions and reacts accordingly with an if/else statement. This repeats until 'while' is met.

My copy:
5 non-static methods and static main. One method controls user input, two control the attack of each of the characters, and two control the health of each of the characters. Main then handles in the return values and chooses how to act according to 1, the others handle local variable changes and math for these variables.

Also, because I learn by doing and exploring in my current field of study I added two menu options that act accordingly and currently thinking about modifying the do/while to incorporate a kill command from the player to exit the program, otherwise the monster will increment in power until the player is 1 shot. Also thinking about critical strikes and misses using Random();

I will greatly, greatly appreciate any help I get with this.

##### Share on other sites
well, in my opinion, your code is quite readable. i would have done some things in a different way but overall it is OK. one more thing i'd like to add and again this is Just My Opinion, others may disagree with me - the code you write should be self-explanatory, writing one line comments(e.g., // comment here)should be used rarely , better if never! i myself prefer using XML comments

##### Share on other sites
Ahhh, ok. I always read stories about someone dredging up other files or reading other peoples work where confusion ensues. I just wanted to ensure any bad habits were curbed before they started.

Secondly, what would you have done differently? And again, thank you for your input.

##### Share on other sites
A few things you might want to look into.

With your switch statement, using the string 'choice'. Make it choice.ToUpper or choice.ToLower, then you can get rid of all the upper or lower case options. Just makes it easier if you are adding more commands.

Next, on to Rand. You really only want ONE rand in your whole app. If you keep creating it like you do it can have the same seed value and give you the same value more then once. I normally create a "Dice" static class that holds my only random. You can also create on and pass it around. I prefer the static because it doesn't change so no real reason not to go static.

Will look for more later.

##### Share on other sites
What would be the most recommended way to seed Rand? I read ages past to use system time or something like that...but I just need to figure out how to do it.

Also for using choice.ToUpper & choice.ToLower, what is the general syntax and usage of that? That's my first run in with sub-variable variables, or at least how I understand them at first look.

##### Share on other sites

What would be the most recommended way to seed Rand? I read ages past to use system time or something like that...but I just need to figure out how to do it.

Also for using choice.ToUpper & choice.ToLower, what is the general syntax and usage of that? That's my first run in with sub-variable variables, or at least how I understand them at first look.

switch(choice.ToUpper())

As for the Rand, you really don't need to seed it, because if you don't use a seed it uses the current system time. What you need to do is use only ONE rand in your whole app.

##### Share on other sites

Next, on to Rand. You really only want ONE rand in your whole app. If you keep creating it like you do it can have the same seed value and give you the same value more then once. I normally create a "Dice" static class that holds my only random. You can also create on and pass it around. I prefer the static because it doesn't change so no real reason not to go static.

Remaking rand was the biggest thing that stood out for me too. On your second one, you should try to make it object oriented. Make a player class and a monster class and use them to hold all the health/attack/whatever else and then your game loop would just look like:

pseudo-code
 do{ attacker.Attack(defender); if(GetDefender.isDead) stop running; break; Player temp = attacker; attacker = defender; defender = temp; } while ( game is running); 

imo it's a good rule of thumb to keep your main loop as simple as possible.

##### Share on other sites
Just from a quick glance I'd put attackChoice as an enumerated type. I'm sure there are functions so you can actually display the object name, so you never have a manual conversion between values. Take's a bit more time to set up initially, but for me the beauty of c# is that you can so heavily type everything that you can get the compiler to do a huge amount of the validation of your code. I'm just learning myself, but its amazing how much of the time that when you've got it to compile, it actually does what you want it to do first time.

##### Share on other sites
Yeah don't get into the habit of making dependencies (like a shared Random) static. Read the wiki article on the SOLID principles (specifically Dependency Inversion) for an explanation as to why.

It looks like you haven't programmed much so try to get into good habits : )

##### Share on other sites
Yeah, that's why I'm here. I'm trying to get bad habits stopped before they start. Thanks for all the help so far!.

1. 1
2. 2
3. 3
Rutin
16
4. 4
5. 5

• 12
• 26
• 10
• 11
• 9
• ### Forum Statistics

• Total Topics
633723
• Total Posts
3013545
×