patterncsharpMinor
C# program that uses events
Viewed 0 times
usesprogramthatevents
Problem
I am trying to learn C#. Please tell me if the following code respects coding standards and naming conventions because I want to write beautiful code. The purpose of this program was to help me understand different concepts that I've learned in the last few days but I was having difficulties thoroughly understanding them (especially events and delegates). Please show me what you would change by exemplifying, not just by recommending, because I have a harder time understanding phrases than code.
For example, I've been told to use "async/await and the TPL". I've made a couple simple programs with them, yet I still don’t know how I would apply the concepts here. I'll keep trying while waiting for an answer from you, I'll be reading the chapters from here in the next few hours.
Also, I've been told I should have used
TProgram.cs:
```
#define DEBUG
#define TRACE
using System;
using System.Diagnostics;
using System.Text;
using System.Threading;
namespace LearnEventsAndOthers
{
class Program
{
private static void OnTicked(object sender, TickingEventArgs e)
{
Console.WriteLine("Event triggered in object {0} at tick {1}", ((Metronom)sender).Id, e.TickCount);
}
static int Main(string[] args)
{
#region Standard Code - Init
Console.InputEncoding = Console.OutputEncoding = Encoding.UTF8;
Console.SetWindowSize(160, 56);
Console.BufferHeight = 9000;
Console.BufferWidth = 300;
#if DEBUG
Console.WriteLine("The program is running in #DEBUG mode.");
#endif
#if TRACE
Console.WriteLine("The program is running in #TRACE mode.");
#endif
#endregion
// Creating objects and testing overridden functions Equals and GetHashCode...
Metronom m = new Metronom();
Metronom n = new Metrono
For example, I've been told to use "async/await and the TPL". I've made a couple simple programs with them, yet I still don’t know how I would apply the concepts here. I'll keep trying while waiting for an answer from you, I'll be reading the chapters from here in the next few hours.
Also, I've been told I should have used
Action. I want to use Actions because I like Action and Func, yet I don’t know where should I use Action.TProgram.cs:
```
#define DEBUG
#define TRACE
using System;
using System.Diagnostics;
using System.Text;
using System.Threading;
namespace LearnEventsAndOthers
{
class Program
{
private static void OnTicked(object sender, TickingEventArgs e)
{
Console.WriteLine("Event triggered in object {0} at tick {1}", ((Metronom)sender).Id, e.TickCount);
}
static int Main(string[] args)
{
#region Standard Code - Init
Console.InputEncoding = Console.OutputEncoding = Encoding.UTF8;
Console.SetWindowSize(160, 56);
Console.BufferHeight = 9000;
Console.BufferWidth = 300;
#if DEBUG
Console.WriteLine("The program is running in #DEBUG mode.");
#endif
#if TRACE
Console.WriteLine("The program is running in #TRACE mode.");
#endif
#endregion
// Creating objects and testing overridden functions Equals and GetHashCode...
Metronom m = new Metronom();
Metronom n = new Metrono
Solution
c
l
m
n
_t
These are poorly named variables that don't follow what most developers have accepted as the norm.
You need to be more descriptive with your names, even when writing Generic code.
c->clockortimerorstopwatch1
That brings me to
l.... You don't need it and shouldn't use it, it's a Magical Variable that is just smelly. You should instead write it like this
private void Clock()
{
Stopwatch clock = new Stopwatch();
clock.Start();
Debug.WriteLineIf(_t.IsAlive, string.Format("Thread from object {0} has gone LIVE!", this.Id));
while (!_shouldStop)
{
if ((clock.ElapsedTicks % this.RandomStop) == 0)
{
OnTickedEvent(clock.ElapsedTicks);
}
}
}Edit:
I see your point that you need that magic smelly variable to hold the exact value, but what if the timing is just right so that
l % this.RandomStop != 0Just a thought.
Edit:
Now there is also an issue with your boolean
_shouldStop and that you don't initialize it to a value, it may not cause a direct issue because if a boolean is not initialized it should return a false value when referenced (IIRC), but this is not a good habit to get into, if it were another object type it would return an error the first time that Clock is called.Easy solution, since you don't want it stopped from the get go just set the variable on declaration
private volatile bool _shouldStop = false;m and n are Metronomes, so their names should reflect that, each one is a Metronom.Since this is a simple method for testing the class that you made I would just name them
metronome1 and metronome2.If they served a specific purpose you could name them something more descriptive.
And that brings me to the last one
_tOkay, it's a thread.
- Is it my thread?
- Is it your thread?
- Is it the main thread?
- Is it scope specific?
- Is this thread safe?
- What is the purpose of this thread?
You should be much more descriptive with your naming.
As a Developer you write code, it should read like a book, so write it elegantly and don't be skimpy.
When I read code and see a single letter variable I have to stop looking at the code and figure out what that letter represents because the name doesn't tell me anything about the object.
Additional:
I noticed that you declare your
Main method as an integer method... not sure why you do that just to return 0;You should just declare it
void there isn't any reason that I can see that you need an integer method.Code Snippets
private void Clock()
{
Stopwatch clock = new Stopwatch();
clock.Start();
Debug.WriteLineIf(_t.IsAlive, string.Format("Thread from object {0} has gone LIVE!", this.Id));
while (!_shouldStop)
{
if ((clock.ElapsedTicks % this.RandomStop) == 0)
{
OnTickedEvent(clock.ElapsedTicks);
}
}
}l % this.RandomStop != 0private volatile bool _shouldStop = false;Context
StackExchange Code Review Q#70208, answer score: 6
Revisions (0)
No revisions yet.