• Create Account

Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

12 replies to this topic

### #1macho  Members   -  Reputation: 140

Like
3Likes
Like

Posted 12 December 2013 - 10:24 AM

Is it alright to post code here to be reviewed for advice?  I'm just starting so its nothing complex but I'd like to make sure I'm not making any early mistakes or slipping up.

I'm taking the Beginning Game Programming with C# on Coursera (its free and open to all if anyone is interested https://class.coursera.org/gameprogramming-001/class/index) but I started after the class had finished and can't get any feedback on my work.

Appreciate the time and attention.  First bit is posted below.  If its fine and/or there is a better place to put this kind of stuff (was thinking a journal?) please let me know.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Assessment1Score
{
/// <summary>
/// Print a welcome message to the user telling them that this application will calculate their average gold-collecting performance.
/// Prompt user for the total gold they've collected in game.
/// Read in total gold collected and put the value into a variable of the appropriate type.
/// Prompt user for total number of hours they've played.
/// Read in the total number of hours played and put the value into a variable of the appropriate type.
/// Convert the hours to minutes and put the value into a variable of the appropriate type.
/// Calculate gold per minute statistic and put the result into a variable of the appropriate type.
/// Print out the gold, hours played, and gold per minute statistic.
/// </summary>
class Program
{
static void Main(string[] args)
{
int gold;
float hoursPlayed;
float minutesPlayed;
float goldPerMinute;

Console.WriteLine("Welcome, this program will calculate the average amount of gold you've earned over your adventures!");
Console.Write("Please input the amount of gold you currently have: ");
Console.Write("Please input the amount of hours you've played: ");
minutesPlayed = hoursPlayed * 60;
goldPerMinute = (float)gold / minutesPlayed;
Console.WriteLine("Your current amount of gold is: " + gold);
Console.WriteLine("You have played for " + hoursPlayed + " hours.");
Console.WriteLine("Your gold per minute score is: " + goldPerMinute);
Console.WriteLine("Thanks for using this calculation program!");

}
}
}



### #2kiteflyingmonkey  Members   -  Reputation: 308

Like
6Likes
Like

Posted 12 December 2013 - 10:43 AM

Looks good, there is nothing wrong with it that I can see.
It's very neat and easy to read, nice one.

Something to consider when writing a program that takes user input is "What is the stupidest thing someone could do at this point?".
What does float.Parse do when you give it something that can't possibly be a float?
Does it make sense if the user inputs a negative time? Perhaps you could stop them...

Keep it up

### #3macho  Members   -  Reputation: 140

Like
0Likes
Like

Posted 12 December 2013 - 11:07 AM

I appreciate your feedback.  I'd assume using if statements would solve that, but I haven't made it that far yet.  I worked with them a bit in JavaScript when I took web programming a bit ago.

I'm not sure yet what the C# equivalent of NaN is but I'd go with using

if (variable = NaN)
Console.WriteLine("Not a number.");


or something in that ballpark.  That's all stuff I've barely seen in action though.

Again, I really, truly appreciate the feedback!

### #4tharealjohn  Members   -  Reputation: 451

Like
2Likes
Like

Posted 12 December 2013 - 11:31 AM

Looks fine to me.

You could also use TryParse instead of just plain Parse. It returns a bool indicating wether the parse was successful. i.e it would return false when trying to parse something thats NaN.

if(!int.TryParse(Console.ReadLine(), out gold))
Console.WriteLine("Not a number.");

Edited by tharealjohn, 12 December 2013 - 11:31 AM.

jmillerdev.com

### #5fastcall22  Crossbones+   -  Reputation: 2722

Like
6Likes
Like

Posted 12 December 2013 - 11:39 AM

In C#, any type with Parse usually has a TryParse as well. If Parse fails, it throws an execption, if TryParse fails, it returns false:

int gold = 0;
bool valid = false;
do {
Console.Write("Please input the amount of gold you currently have: ");
try {
valid = true;
} catch ( Exception ex ) {
Console.Write("Error: ");
Console.WriteLine( ex.Message );
}
} while ( !valid );

int gold = 0;
do {
Console.Write("Please input the amount of gold you currently have: ");
if ( !Int32.TryParse(Console.ReadLine(),out gold) ) {   // use "out" to pass gold by reference; TryParse will fill in this variable
continue;
}
break;
} while ( true );


### #6macho  Members   -  Reputation: 140

Like
4Likes
Like

Posted 12 December 2013 - 12:14 PM

Ah, that makes sense.  I haven't seen some of those statements yet ( try/catch etc.). But I get the idea.

I suppose if I did:

int gold = 0;
do {
Console.Write("Please input the amount of gold you currently have: ");
if ( !Int32.TryParse(Console.ReadLine(),out gold) || gold <= 0 ) {   // use "out" to pass gold by reference; TryParse will fill in this variable
continue;
}
break;
} while ( true );


That would catch instances of non-integer and negative integer values in one swoop?

I appreciate you guys pointing this out to me.  I haven't touched loops or anything but I do grasp the ideas behind them.

Edit:  I misread the if statement to testing if true instead of testing if false, changed && >= 0 to || <=0

Edited by macho, 12 December 2013 - 12:15 PM.

### #7AlanSmithee  Members   -  Reputation: 884

Like
4Likes
Like

Posted 12 December 2013 - 03:30 PM

I appreciate your feedback.  I'd assume using if statements would solve that, but I haven't made it that far yet.  I worked with them a bit in JavaScript when I took web programming a bit ago.

I'm not sure yet what the C# equivalent of NaN is but I'd go with using

if (variable = NaN)
Console.WriteLine("Not a number.");


or something in that ballpark.  That's all stuff I've barely seen in action though.

Again, I really, truly appreciate the feedback!

Not sure if that was just an example, orif you actually use that code snippet, but if you do, be sure too use "equal to" == instead of "assign operator" =.

leftHandValue == rightHandValue "is the left had value equal to the right hand value?"

leftHandValue = rightHandValue "assign the right hand value to the left hand value"

you probably know this, but just to make it clear.

### #8macho  Members   -  Reputation: 140

Like
1Likes
Like

Posted 12 December 2013 - 03:51 PM

I wasn't using it but I certainly would have messed that up!  Thank you for the catch and correction, I'll pay attention to it in the future.

Edit with new code!  And another to remove a paragraph I copied into the code block..oops.

This one was fun and was the first primer to objects and classes.  I feel as though I 70% get the idea.  I'm not quite sure what Members means in regards to a class.  I get that Methods are essentially the actions of an object and properties store data for an object.  That's probably simplifying them a good deal though.  I haven't made any classes or objects yet, so I don't know their inner workings.

I'm hoping more exposure to them will clear up the blank spots.

This one was fun and was the first primer to objects and classes. I feel as though I 70% get the idea. I'm not quite sure what Members means in regards to a class. I get that Methods are essentially the actions of an object and properties store data for an object. That's probably simplifying them a good deal though. I haven't made any classes or objects yet, so I don't know their inner workings.

I'm hoping more exposure to them will clear up the blank spots.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Lab4
{
/// <summary>
/// The Deck class is provided.
/// </summary>
class Program
{
static void Main(string[] args)
{
// create a new deck and print the contents of the deck
Deck newDeck = new Deck();

newDeck.Print();

Console.WriteLine("///////////////////*Shuffle Seperator*\\\\\\\\\\\\\\\\");

// shuffle the deck and print the contents of the deck

newDeck.Shuffle();
newDeck.Print();

Console.WriteLine("///////////////////*Top Card Seperator*\\\\\\\\\\\\\\\\");

// take the top card from the deck and print the card rank and suit
Card topCard = newDeck.TakeTopCard();
Console.WriteLine("The top card was the " + topCard.Rank + " of " + topCard.Suit);

// take the top card from the deck and print the card rank and suit
topCard = newDeck.TakeTopCard();
Console.WriteLine("The top card was the " + topCard.Rank + " of " + topCard.Suit);

}
}
}



Edited by macho, 12 December 2013 - 07:05 PM.

### #9boogyman19946  Members   -  Reputation: 932

Like
0Likes
Like

Posted 12 December 2013 - 10:40 PM

I suppose you wouldn't know the difference between a free function and a method as I think C# is purely OOP (I know Java is for example).

The word member means to be a part of. In terms of a class, you say "member variable of ClassA" meaning that variable is contained by ClassA. The same thing with functions. A member function, otherwise known as a method, is a function that's a part of that class. It has access to all members of that class and is a means for outside code to work with the data of that class (preferably without exposing it). If I remember correctly, C# does not allow freestanding functions, which means all you get to work with are methods.

Methods are special because they add an extra "this" pointer to the argument list that you can't see is there, but perhaps it's best not to confuse things early on

Edited by boogyman19946, 12 December 2013 - 10:41 PM.

"If highly skilled generalists are rare, though, then highly skilled innovators are priceless." - ApochPiQ

- Java API Documentation - For all your Java info needs
- C++ Standard Library Reference - For some of your C++ needs ^.^

### #10jHaskell  Members   -  Reputation: 667

Like
1Likes
Like

Posted 13 December 2013 - 07:46 AM

I appreciate your feedback.  I'd assume using if statements would solve that, but I haven't made it that far yet.  I worked with them a bit in JavaScript when I took web programming a bit ago.

I'm not sure yet what the C# equivalent of NaN is but I'd go with using

if (variable = NaN)
Console.WriteLine("Not a number.");


or something in that ballpark.  That's all stuff I've barely seen in action though.

Again, I really, truly appreciate the feedback!

Not sure if that was just an example, orif you actually use that code snippet, but if you do, be sure too use "equal to" == instead of "assign operator" =.

leftHandValue == rightHandValue "is the left had value equal to the right hand value?"

leftHandValue = rightHandValue "assign the right hand value to the left hand value"

you probably know this, but just to make it clear.

C# doesn't allow assignments within comparison blocks.  That code will cause a compilation error.

### #11AlanSmithee  Members   -  Reputation: 884

Like
0Likes
Like

Posted 13 December 2013 - 01:03 PM

I appreciate your feedback.  I'd assume using if statements would solve that, but I haven't made it that far yet.  I worked with them a bit in JavaScript when I took web programming a bit ago.

I'm not sure yet what the C# equivalent of NaN is but I'd go with using

if (variable = NaN)
Console.WriteLine("Not a number.");


or something in that ballpark.  That's all stuff I've barely seen in action though.

Again, I really, truly appreciate the feedback!

Not sure if that was just an example, orif you actually use that code snippet, but if you do, be sure too use "equal to" == instead of "assign operator" =.

leftHandValue == rightHandValue "is the left had value equal to the right hand value?"

leftHandValue = rightHandValue "assign the right hand value to the left hand value"

you probably know this, but just to make it clear.

C# doesn't allow assignments within comparison blocks.  That code will cause a compilation error.

I did not know that.. thanks for sharing.. guess I haven't done that typo in C# yet ;) (probably thanks too the asweome intellisense of VS rather than me screwing up the syntax.. i know that happens way too often anyway xD )

### #12Nypyren  Crossbones+   -  Reputation: 3278

Like
2Likes
Like

Posted 13 December 2013 - 01:53 PM

C# doesn't allow assignments within comparison blocks.  That code will cause a compilation error.

This is not entirely true. C# allows assignments in comparisons. However, the comparison expression must result in a boolean (which is what catches most of the typos). This means you can still be affected by the typo, but only with boolean variables:

bool b = something;
bool c = somethingElse;
if (b = c)
Console.WriteLine("Hi"); // This is printed if c is true, regardless of what b was.


Edited by Nypyren, 13 December 2013 - 01:55 PM.

### #13Paragon123  Members   -  Reputation: 301

Like
0Likes
Like

Posted 17 December 2013 - 11:01 AM

With c# I wouldn't concatenate strings the way you are doing it.

For example:

String name="MyName"

Console.WriteLine("Hello, "+name);

should be

Console.WriteLine("Hello, {0}",name);

It tends to be easier to read and easier to modify later.

If WriteLine didn't have that capability built into it try:

Console.WriteLine(String.Format("Hello, {0}",name));

If you are building a string for storage use a StringBuilder

StringBuilder sb=new StringBuilder();

sb.AppendFormat("Hello, {0}",name);

Console.Writeline(sb.ToString());

In C#, each time you concatenate a string it creates a new instance, then copies the two contatenations.

String C="A"+"B"+"C"

creates a String "A" and a String "B", then a new String and copies "A" and "B" to make "AB", then a string "C" then a new String and copies "AB" and "C" to make "ABC"

A StringBuilder dynamically modifies the memory size and avoids excessive copies.

If it is a simple concatenation and you don't need a full on StringBuilder Consider,

String.Concatenate("A","B","C")

For string comparisons, remember

if (String.IsNullOrEmpty(MyString)) Console.Writeline("No text in string")

and use

If (String.Compare(Str1,Str2,true)==0) Console.Writeline("The text is the same, but the casing may be different")

Don't use <type> v = <type>.Parse(String)

use

<type> v;

if (!<type>.TryParse(String, out v)) v=<default>;

-or-

if(!<type>.TryParse(String, out v)) throw new exception("Invalid data.");

Edited by Paragon123, 17 December 2013 - 11:03 AM.

Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

PARTNERS