[VirtualFrost]

Hey guys,

So I had been creating a Notepad application, however I felt suddenly unmotivated to continue on with it, so I thought I'd work on a new project, something easy to keep me going. I made this all today, probably spent a total of 4 hours which is a lot more than I've spent.

I was hoping to receive feedback, maybe if I could have done something more efficiently? I know I've used a lot of If/Else statements so maybe there is a way to remove those with something more effective. You can also try break it if you'd like to, but overall this is my first 'finished' application in C#.

Github Link
Virus Total Scan

Download is inside the Github, even though the source is there, I'll provide you guys with a VirusTotal scan as well. Not sure how it actually received a hit for Malware, maybe I did something wrong in my code?

[My Computer]

Not bad, only suggestion I have for you is when using the Random class, don't create a new instance of it every time you need it. Instead you should only create one instance of it which can be accessed globally.

Example of what I mean:

Code

class Program
    {
        public static Random random;

        static void Main(string[] args)
        {
            random = new Random();
            int randNumber = RandomNumber(0, 10);

            Console.WriteLine("Your random number is: " + randNumber);
            Console.ReadKey();
        }

        public static int RandomNumber(int minValue, int maxValue)
        {
            return random.Next(minValue, maxValue);
        }
    }

Instead of:

Code

public static int RandomNumber(int minValue, int maxValue)
        {
            Random rand = new Random();
            int randNumber = rand.Next(minValue, maxValue);
            return randNumber;
        }

But anyways, good job on making this. 👍

[VirtualFrost]

Not bad, only suggestion I have for you is when using the Random class, don't create a new instance of it every time you need it. Instead you should only create one instance of it which can be accessed globally.

Example of what I mean:

Code

class Program
    {
        public static Random random;

        static void Main(string[] args)
        {
            random = new Random();
            int randNumber = RandomNumber(0, 10);

            Console.WriteLine("Your random number is: " + randNumber);
            Console.ReadKey();
        }

        public static int RandomNumber(int minValue, int maxValue)
        {
            return random.Next(minValue, maxValue);
        }
    }

Instead of:

Code

public static int RandomNumber(int minValue, int maxValue)
        {
            Random rand = new Random();
            int randNumber = rand.Next(minValue, maxValue);
            return randNumber;
        }

But anyways, good job on making this. 👍

I think I understand what you mean, so basically define it globally, and then access it/change it instead of recreating it each time I hit the New Game button?
Thank you for the feedback!

[Zilog80]

Pro-tip #1:
No need compare boolean to "true" (or false):

PHP Code


<?php 
if (easyGuessMenuItem.Checked == true)
{
    userDifficulty = 10;
} 


Can be coded as:

PHP Code


<?php 
if (easyGuessMenuItem.Checked)
    userDifficulty = 10;
else if (mediumGuessMenuItem.Checked)
    userDifficulty = 7;
... 


Pro-tip #2:

That @My Computer said

Hope it helps 🤝

[VirtualFrost]

Pro-tip #1:
No need compare boolean to "true" (or false):

PHP Code


<?php 
if (easyGuessMenuItem.Checked == true)
{
    userDifficulty = 10;
} 


Can be coded as:

PHP Code


<?php 
if (easyGuessMenuItem.Checked)
    userDifficulty = 10;
else if (mediumGuessMenuItem.Checked)
    userDifficulty = 7;
... 


Pro-tip #2:

That @My Computer said

Hope it helps 🤝

Fuck, that actually makes so much sense, I'm not sure why I did that, thanks a lot! Fixing that up now, will publish it when I work more on it tomorrow hopefully.

[Cecilia]

Kinda weird to see Convert.ToString being used.
Any reason you are using it?

I'd personally do something like:

PHP Code


<?php 
(userDifficulty - userGuess).ToString(); 


Perhaps make an extension method for 'ToInt' or such as well?
Perhaps instead of using menu checkboxes use a sequence of radio buttons?

frmMain/Game would perhaps be a better name for the form?
(I'd do FrmMain as that's my coding style).


Just a few tips anyway, hopefully they help in some way.

Btw with your VT scan don't fret. 360 picks up some .net files without reasons.

[VirtualFrost]

Kinda weird to see Convert.ToString being used.
Any reason you are using it?

I'd personally do something like:

PHP Code


<?php 
(userDifficulty - userGuess).ToString(); 


Convert.ToString is (or now was) the only way I knew how to convert those integers to strings so that they could be used inside my Label, is there a difference between the two, or are you more used to doing it that way?

Perhaps make an extension method for 'ToInt' or such as well?

Could you expand on this? What do you mean by make an extension method for 'ToInt'?

Perhaps instead of using menu checkboxes use a sequence of radio buttons?

I could do that, just with the design at the time I didn't want to start moving everything around as if you saw the screenshots, it's pretty small(for now) so I threw in a menustrip along the top of the application instead.

frmMain/Game would perhaps be a better name for the form?
(I'd do FrmMain as that's my coding style).

Game is an unusual name I do agree, at the time I guess I was rushing to just get the idea down and make it without worrying about the name too much. But if I did redo it, it probably would be something along the lines of formGame. Never seen forms written like you've described actually, but it is easy to read.

Just a few tips anyway, hopefully they help in some way.

Btw with your VT scan don't fret. 360 picks up some .net files without reasons.

Alright cool, thank you so much!

[Cecilia]

Convert.ToString is (or now was) the only way I knew how to convert those integers to strings so that they could be used inside my Label, is there a difference between the two, or are you more used to doing it that way?

Convert.ToString just simply calls ToString on the value so it is exactly the same.

PHP Code


<?php 
public static string ToString(int value)
{
    return value.ToString(CultureInfo.CurrentCulture);
} 


Could you expand on this? What do you mean by make an extension method for 'ToInt'?

You can create your own 'Extension' classes that you can simply 'stringAsNum.ToInt()'

https://msdn.microsoft.com/en-us/library/bb383977.aspx


Glad to help :)

[Zilog80]

Hehe, glad to help too 😁

Another pro-tip:

Pro-tip #3: i'm agree with @LaPanthere that radio buttons would be more suitable. But anyway if you still use menu toolstrips (not bad at all) you could change your clearMenuChecked to:

PHP Code


<?php 
private void clearMenuChecked()
{
    foreach (ToolStripMenuItem ítem in difficultyToolStripItem.DropDownItems)
    {
        item.Checked = false;
    }
} 


This for-each loop set checked to false to all difficultyToolStripItem items. So doesn't matters how many difficulty levels you have.

Pro-tip #4: if you change your version (Current Version: 1.1.0) you should change your assembly info:

Spoiler (Click to View)



After changing As spoiler you get "real version":



Hope it helps :)

[Cecilia]

After changing As spoiler you get "real version":



Hope it helps :)

Just remember, the IDE has a thing on Project Properties -> Application -> Assembly Information...

https://i.imgur.com/SKjduMq.png

[Zilog80]

Just remember, the IDE has a thing on Project Properties -> Application -> Assembly Information...

https://i.imgur.com/SKjduMq.png

Yeah, thats true. ✌

(I always have solution explorer open and personally always edited that file manually, but yes, @OP can use the IDE feature you posted)

[VirtualFrost]

Convert.ToString just simply calls ToString on the value so it is exactly the same.

PHP Code


<?php 
public static string ToString(int value)
{
    return value.ToString(CultureInfo.CurrentCulture);
} 


You can create your own 'Extension' classes that you can simply 'stringAsNum.ToInt()'

https://msdn.microsoft.com/en-us/library/bb383977.aspx


Glad to help :)

I'll work on this once I get home!

Hehe, glad to help too 😁

Another pro-tip:

Pro-tip #3: i'm agree with @LaPanthere that radio buttons would be more suitable. But anyway if you still use menu toolstrips (not bad at all) you could change your clearMenuChecked to:

PHP Code


<?php 
private void clearMenuChecked()
{
    foreach (ToolStripMenuItem ítem in difficultyToolStripItem.DropDownItems)
    {
        item.Checked = false;
    }
} 


This for-each loop set checked to false to all difficultyToolStripItem items. So doesn't matters how many difficulty levels you have.

Pro-tip #4: if you change your version (Current Version: 1.1.0) you should change your assembly info:

Spoiler (Click to View)



After changing As spoiler you get "real version":



Hope it helps :)

I'm surprised with the version thing because for some reason when I googled it, there was this really long guide, so not even sure what that was for anymore. Thank you for showing me this, I needed it!