Skip to main content
Code Review

Return to Answer

deleted 2 characters in body
Source Link
Heslacher
Heslacher
  • 50.4k
  • 5
  • 81
  • 175

The rest of it should be implemented in the InitializeClock method that now reqruiesrequires a startTime and creates an instance of a Clock if everything went fine:

The last change is inside the clock. Instead of running a while(true) loop you should use a Timer that will tick every second and you can either count the seconds with a counter (like I did) or modify the DateTime field. You can do this with the AddSeconds method which returns a new instance of the DateTime sturctstruct so you'll need to save this result or like I did just display the new time with some formatting.

class Clock
{
    private readonly DateTime _startTime;
    private double _offset;
    private readonly System.Timers.Timer _timer;
    public Clock(DateTime startTime)
    {
        _startTime = startTime;
        _timer = new System.Timers.Timer(1000);
        _timer.Elapsed += (sender, e) =>
        {
            _offset++;
            DisplayTime();
        };
        _timer.Start();
    }

    public void DisplayTime()
    { 

        Console.WriteLine(_startTime.AddSeconds(_offset).ToString("hh:mm:ss"));
    }
}

Console.WriteLine(_startTime.AddSeconds(_offset).ToString("hh:mm:ss")); } }

The rest of it should be implemented in the InitializeClock method that now reqruies a startTime and creates an instance of a Clock if everything went fine:

The last change is inside the clock. Instead of running a while(true) loop you should use a Timer that will tick every second and you can either count the seconds with a counter (like I did) or modify the DateTime field. You can do this with the AddSeconds method which returns a new instance of the DateTime sturct so you'll need to save this result or like I did just display the new time with some formatting.

class Clock
{
    private readonly DateTime _startTime;
    private double _offset;
    private readonly System.Timers.Timer _timer;
    public Clock(DateTime startTime)
    {
        _startTime = startTime;
        _timer = new System.Timers.Timer(1000);
        _timer.Elapsed += (sender, e) =>
        {
            _offset++;
            DisplayTime();
        };
        _timer.Start();
    }

    public void DisplayTime()
    {

Console.WriteLine(_startTime.AddSeconds(_offset).ToString("hh:mm:ss")); } }

The rest of it should be implemented in the InitializeClock method that now requires a startTime and creates an instance of a Clock if everything went fine:

The last change is inside the clock. Instead of running a while(true) loop you should use a Timer that will tick every second and you can either count the seconds with a counter (like I did) or modify the DateTime field. You can do this with the AddSeconds method which returns a new instance of the DateTime struct so you'll need to save this result or like I did just display the new time with some formatting.

class Clock
{
    private readonly DateTime _startTime;
    private double _offset;
    private readonly System.Timers.Timer _timer;
    public Clock(DateTime startTime)
    {
        _startTime = startTime;
        _timer = new System.Timers.Timer(1000);
        _timer.Elapsed += (sender, e) =>
        {
            _offset++;
            DisplayTime();
        };
        _timer.Start();
    }

    public void DisplayTime()
    { 

        Console.WriteLine(_startTime.AddSeconds(_offset).ToString("hh:mm:ss"));
    }
}
added 933 characters in body
Source Link
t3chb0t
t3chb0t
  • 44.3k
  • 9
  • 84
  • 190

You use the Date property to format the date. This is not necessary and you can format any part of a date time with the appropriate format string, see String Format for DateTime for many more examples.

The last change is inside the clock. Instead of running a while(true) loop you should use a Timer that will tick every second and you can either count the seconds with a counter (like I did) or modify the DateTime field. You can do this with the AddSeconds method which returns a new instance of the DateTime sturct so you'll need to save this result or like I did just display the new time with some formatting.

This is just an example and the clock should not actually write to the console itself. You might want to pass it a service via DI or you might have an entirely different idea depending on your actual solution.

class Clock
{
    private readonly DateTime _startTime;
    private double _offset;
    private readonly System.Timers.Timer _timer;
    public Clock(DateTime startTime)
    {
        _startTime = startTime;
        _timer = new System.Timers.Timer(1000);
        _timer.Elapsed += (sender, e) =>
        {
            _offset++;
            DisplayTime();
        };
        _timer.Start();
    }

    public void DisplayTime()
    {
        Console.WriteLine(_startTime.AddSeconds(_offset).ToString("hh:mm:ss"));
    }
}

Console.WriteLine(_startTime.AddSeconds(_offset).ToString("hh:mm:ss")); } }

In this example I'm using an anonymous function for the event handler. There are also other techniques. You can read more about them on How to: Subscribe to and Unsubscribe from Events .

The new Main would then just pass the time string to the DisplayTimeInitializeClock method but since GetTime is async and Main is not we need to use Task.Run to be able to await it:

Task.Run(async () =>
{
    var time = await GetTime();
Task.Run(async () =>  InitializeClock(await time);
});

The last change is inside the clock. Instead of running a while(true) loop you should use a Timer that will tick every second. This is just an example and the clock should not actually write to the console itself. You might want to pass it a service via DI or you might have an entirely different idea depending on your actual solution.

class Clock
{
    private readonly DateTime _startTime;
    private double _offset;
    private readonly System.Timers.Timer _timer;
    public Clock(DateTime startTime)
    {
        _startTime = startTime;
        _timer = new System.Timers.Timer(1000);
        _timer.Elapsed += (sender, e) =>
        {
            _offset++;
            DisplayTime();
        };
        _timer.Start();
    }

    public void DisplayTime()
    {
        Console.WriteLine(_startTime.AddSeconds(_offset).ToString("hh:mm:ss"));
    }
}

The new Main would then just pass the time string to the DisplayTime method:

var time = GetTime();
Task.Run(async () => InitializeClock(await time));

You use the Date property to format the date. This is not necessary and you can format any part of a date time with the appropriate format string, see String Format for DateTime for many more examples.

The last change is inside the clock. Instead of running a while(true) loop you should use a Timer that will tick every second and you can either count the seconds with a counter (like I did) or modify the DateTime field. You can do this with the AddSeconds method which returns a new instance of the DateTime sturct so you'll need to save this result or like I did just display the new time with some formatting.

This is just an example and the clock should not actually write to the console itself. You might want to pass it a service via DI or you might have an entirely different idea depending on your actual solution.

class Clock
{
    private readonly DateTime _startTime;
    private double _offset;
    private readonly System.Timers.Timer _timer;
    public Clock(DateTime startTime)
    {
        _startTime = startTime;
        _timer = new System.Timers.Timer(1000);
        _timer.Elapsed += (sender, e) =>
        {
            _offset++;
            DisplayTime();
        };
        _timer.Start();
    }

    public void DisplayTime()
    {

Console.WriteLine(_startTime.AddSeconds(_offset).ToString("hh:mm:ss")); } }

In this example I'm using an anonymous function for the event handler. There are also other techniques. You can read more about them on How to: Subscribe to and Unsubscribe from Events .

The new Main would then just pass the time string to the InitializeClock method but since GetTime is async and Main is not we need to use Task.Run to be able to await it:

Task.Run(async () =>
{
    var time = await GetTime();
    InitializeClock(time);
});
Source Link
t3chb0t
t3chb0t
  • 44.3k
  • 9
  • 84
  • 190

foreach (Match match in (new Regex(pattern).Matches(data)))
{
    time = match.Groups[1].Value;
}

The first thing you should change is this loop. You don't need it as there is only one match on that page. Then you can use the static Regex.Match to find the data and you can use a named group instead of the 1 so this could be refactored to a method that actually does nothing else but returning the time string. It should not display the time yet. This means that the return type is not void any more (which should be used only for event handlers anyway) but a Task<string> (in case of a void it should just be Task).

private static async Task<string> GetTime()
{
    Console.WriteLine("Entered getTime");
    try
    {
        varpattern = @"<tr><td colspan=2 align=""center"" bgcolor=""#FFFFFF""><h5>(?<Data>[^>]*)</td></tr>";
        var getTime = Task.Run(async () =>
        {
            varwebsite = "http://www.worldtimezone.com/time/wtzresult.php?CiID=1225&forma=Find%20Time";
            using (var client = new HttpClient())
            using (var response = await client.GetAsync(website))
            using (var content = response.Content)
            {
                string data = await content.ReadAsStringAsync();
                return Regex.Match(data, pattern).Groups["Data"].Value;
            }
        });
        return await getTime;
    }
    catch (Exception ex)
    {
        Console.WriteLine($"getTime Error: {ex.ToString()}");
        return null;
    }
}

The rest of it should be implemented in the InitializeClock method that now reqruies a startTime and creates an instance of a Clock if everything went fine:

private static void InitializeClock(string startTime)
{
    DateTime datetime;
    if (DateTime.TryParse(startTime, out datetime))
    {
        Console.WriteLine(datetime.ToString("hh:mm:ss"));
        Console.WriteLine(datetime.Date.ToString("dd/MM/yyyy"));
        clock = new Clock(datetime);
    }
    else
    {
        Debug.WriteLine("Invalid Format");
    }
}

The last change is inside the clock. Instead of running a while(true) loop you should use a Timer that will tick every second. This is just an example and the clock should not actually write to the console itself. You might want to pass it a service via DI or you might have an entirely different idea depending on your actual solution.

class Clock
{
    private readonly DateTime _startTime;
    private double _offset;
    private readonly System.Timers.Timer _timer;
    public Clock(DateTime startTime)
    {
        _startTime = startTime;
        _timer = new System.Timers.Timer(1000);
        _timer.Elapsed += (sender, e) =>
        {
            _offset++;
            DisplayTime();
        };
        _timer.Start();
    }

    public void DisplayTime()
    {
        Console.WriteLine(_startTime.AddSeconds(_offset).ToString("hh:mm:ss"));
    }
}

The new Main would then just pass the time string to the DisplayTime method:

var time = GetTime();
Task.Run(async () => InitializeClock(await time));

Now you don't need any additional data structures like the TimeAndDate and splitting the time into pieces and calculate the hour or minute part yourself.