RE: Censure worm 1.0 C# source code 05-01-2013, 05:36 AM
#8
Alright, firstly, the reason why C# is not related to C or C++ is, I'm putting this in spoilers because it's quite a bit of reading:
Good share though
For your own benefit I have a few things for you:
Empty methods are not a good thing, I'm assuming in your designer you are assigning an event handler to this method. Although it's never being used here.
You don't need to evaluate against true here, there would be no need for == true or != false. Try this:
Because this is much better. What you are doing above is checking a condition, which already represents true or false, and comparing it with true:
Exs:
Lets take a look at this assuming, CstrikeIsExists = true, and ConfigIsExists = false.
(Note: obviously that is false overall).
Why go through an extra evaluation though?
If you did:
Assuming that each of these still represents, respectively, true and false (in that order). This would be the exact same thing, but evaluated with one less step:
Also, you are creating a new instance of a timer everytime. Why do that? You could stick with one timer declared as a member variable to the Form1 class, and initialize it's tick event to a method from the form's constructor, and then you would only be using one timer, instead of having multiple other instances of the Timer out on the heap, which have been used once, but get left alone and become useless. The GC has to clean all of that up at some point, so save it the work? If you have timers that need to do different things then you either should be disposing of these instances properly, or making a couple of them as global instances.
SoundPlayer implements the IDisposable interface as well, so you should be disposing of that object.
You also seem to be overusing the try, catch here as a method of error "handling". You should avoid this as much as possible, or do something with the error other than just swallowing it.
Your data class as well, should not be inside the Form1 class technically. There's almost never a good reason to have subclasses in my opinion. Especially if you want to access this from someplace else, it shouldn't be in there.
Spoiler:
*Just kidding. 

Good share though

For your own benefit I have a few things for you:
Code:
private void toolTip1_Popup(object sender, PopupEventArgs e)
{
}Empty methods are not a good thing, I'm assuming in your designer you are assigning an event handler to this method. Although it's never being used here.
Code:
if (CstrikeIsExists == true && ConfigIsExists == true)You don't need to evaluate against true here, there would be no need for == true or != false. Try this:
Code:
if (CstrikeIsExists && ConfigIsExists)Because this is much better. What you are doing above is checking a condition, which already represents true or false, and comparing it with true:
Exs:
- 1) true == true
-> This gets evaluated down to a single true, because true does equal true.
-> true
1) false == true
-> This gets evaluated down to a single false, because false does not equal true.
-> false
Lets take a look at this assuming, CstrikeIsExists = true, and ConfigIsExists = false.
Code:
if (CstrikeIsExists == true && ConfigIsExists == true)Code:
if (true == true && false == true)Code:
if (true && false)(Note: obviously that is false overall).
Why go through an extra evaluation though?

If you did:
Code:
if (CstrikeIsExists && ConfigIsExists)Assuming that each of these still represents, respectively, true and false (in that order). This would be the exact same thing, but evaluated with one less step:
Code:
if (true && false)Also, you are creating a new instance of a timer everytime. Why do that? You could stick with one timer declared as a member variable to the Form1 class, and initialize it's tick event to a method from the form's constructor, and then you would only be using one timer, instead of having multiple other instances of the Timer out on the heap, which have been used once, but get left alone and become useless. The GC has to clean all of that up at some point, so save it the work? If you have timers that need to do different things then you either should be disposing of these instances properly, or making a couple of them as global instances.
SoundPlayer implements the IDisposable interface as well, so you should be disposing of that object.
You also seem to be overusing the try, catch here as a method of error "handling". You should avoid this as much as possible, or do something with the error other than just swallowing it.
Your data class as well, should not be inside the Form1 class technically. There's almost never a good reason to have subclasses in my opinion. Especially if you want to access this from someplace else, it shouldn't be in there.
ArkPhaze
"Object oriented way to get rich? Inheritance"
Getting Started: C/C++ | Common Mistakes
[ Assembly / C++ / .NET / Haskell / J Programmer ]
"Object oriented way to get rich? Inheritance"
Getting Started: C/C++ | Common Mistakes
[ Assembly / C++ / .NET / Haskell / J Programmer ]


![[+]](https://sinister.li/images/modern/collapse_collapsed.png)