Skip to content

Commit

Permalink
Improve code quality across multiple files (#203)
Browse files Browse the repository at this point in the history
- Update `CancelAfterAsync` in `BackgroundQueue.cs` to use `CancelAsync` and improved `TimeoutException` message.
- Modifie `ExecuteAsync` in `BackgroundQueueService.cs` to return immediately on cancellation exceptions.
- Enhance `ExecuteTaskChunk` in `ConcurrentBackgroundQueueService.cs` to throw `InvalidOperationException` for null `TaskQueue`.
- Change exception types in `CronJobServiceExtensions.cs` and `Account.cs` to `InvalidOperationException`.
- Follow dispose pattern in `DelayedFileSystemWatcher.cs` and `CaptchaSvgGenerator.cs` and `CronJobService.cs`
- Implement `IEqualityComparer` in `Angle` and `GermanHoliday` classes.
- Refactor `Location.cs` for better readability.
- Make classes `MaidenheadLocator`, `StringFormatter`, and `Constants` static
- Update comments for clarity and removed redundant code.
  • Loading branch information
axunonb authored Oct 7, 2024
1 parent 7fb8876 commit 4d34af8
Show file tree
Hide file tree
Showing 39 changed files with 267 additions and 181 deletions.
4 changes: 2 additions & 2 deletions Axuno.BackgroundTask/BackgroundQueue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ private static async Task CancelAfterAsync(Func<CancellationToken, Task> startTa
// - Either the original task completed, so we need to cancel the delay task.
// - Or the timeout expired, so we need to cancel the original task.
// Canceling will not affect a task, that is already completed.
timeoutCancellation.Cancel();
await timeoutCancellation.CancelAsync();
if (completedTask == originalTask)
{
// original task completed
Expand All @@ -124,7 +124,7 @@ private static async Task CancelAfterAsync(Func<CancellationToken, Task> startTa
else
{
// timeout
throw new TimeoutException();
throw new TimeoutException($"Tasked timed out after {timeout.TotalMilliseconds}ms.");
}
}
}
1 change: 0 additions & 1 deletion Axuno.BackgroundTask/BackgroundQueueService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
catch (Exception e) when (e is TaskCanceledException || e is OperationCanceledException)
{
_logger.LogError(e, $"{nameof(BackgroundQueueService)} canceled.");
//break;
return;
}
catch (Exception e)
Expand Down
6 changes: 4 additions & 2 deletions Axuno.BackgroundTask/ConcurrentBackgroundQueueService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,10 @@ private async Task ExecuteTaskChunk(Queue<IBackgroundTask> taskListReference, Ca
stoppingToken.ThrowIfCancellationRequested();

using var taskCancellation = CancellationTokenSource.CreateLinkedTokenSource(stoppingToken);
var task = (TaskQueue?.RunTaskAsync(taskListReference.Dequeue(), taskCancellation.Token)) ??
throw new NullReferenceException($"{nameof(TaskQueue)} cannot be null here.");
if (TaskQueue == null)
throw new InvalidOperationException($"{nameof(TaskQueue)} must not be null.");

var task = TaskQueue.RunTaskAsync(taskListReference.Dequeue(), taskCancellation.Token);

if (task.Exception != null)
throw task.Exception;
Expand Down
20 changes: 17 additions & 3 deletions Axuno.BackgroundTask/CronJobService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ namespace Axuno.BackgroundTask;
public abstract class CronJobService : IHostedService, IDisposable
{
private System.Timers.Timer? _timer;
private bool _isDisposing;
private readonly CronExpression _expression;
private readonly TimeZoneInfo _timeZoneInfo;

Expand Down Expand Up @@ -72,9 +73,22 @@ public virtual async Task StopAsync(CancellationToken cancellationToken)
await Task.CompletedTask;
}

public virtual void Dispose()
protected virtual void Dispose(bool disposing)
{
_timer?.Dispose();
if (!_isDisposing)
{
if (disposing)
{
_timer?.Dispose();
}

_isDisposing = true;
}
}

public void Dispose()
{
Dispose(disposing: true);
GC.SuppressFinalize(this);
}
}
}
6 changes: 3 additions & 3 deletions Axuno.BackgroundTask/CronJobServiceExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ public static IServiceCollection AddCronJob<T>(this IServiceCollection services,
{
if (options == null)
{
throw new ArgumentNullException(nameof(options), @"Please provide Schedule Configurations.");
throw new ArgumentNullException(nameof(options), "Please provide Schedule Configurations.");
}
var config = new ScheduleConfig<T>();
options.Invoke(config);
if (string.IsNullOrWhiteSpace(config.CronExpression))
{
throw new ArgumentNullException(nameof(ScheduleConfig<T>.CronExpression), @"Empty Cron Expression is not allowed.");
throw new InvalidOperationException($"Empty {nameof(ScheduleConfig<T>.CronExpression)} is not allowed.");
}

services.AddSingleton<IScheduleConfig<T>>(config);
Expand All @@ -33,4 +33,4 @@ public class ScheduleConfig<T> : IScheduleConfig<T>
{
public string CronExpression { get; set; } = string.Empty;
public TimeZoneInfo TimeZoneInfo { get; set; } = TimeZoneInfo.Utc;
}
}
41 changes: 26 additions & 15 deletions Axuno.Tools/FileSystem/DelayedFileSystemWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public class DelayedFileSystemWatcher : IDisposable
private ArrayList _events = new();

private int _consolidationInterval = 1000; // initial value in milliseconds
private bool _isDisposing;
private readonly System.Timers.Timer _timer;

#region *** Delegate to FileSystemWatcher ***
Expand Down Expand Up @@ -203,15 +204,6 @@ public void BeginInit()
_fileSystemWatcher.BeginInit();
}

/// <summary>
/// Releases the unmanaged resources used by the <see cref="FileSystemWatcher"/> and optionally releases the managed resources.
/// </summary>
public void Dispose()
{
_fileSystemWatcher.Dispose();
_timer.Dispose();
}

/// <summary>
/// Ends the initialization of a <see cref="FileSystemWatcher"/> used on a form or used by another component. The initialization occurs at run time.
/// </summary>
Expand Down Expand Up @@ -270,7 +262,7 @@ protected void OnRenamed(RenamedEventArgs e)
/// </summary>
/// <param name="changeType">The <see cref="System.IO.WatcherChangeTypes" /> to watch for.</param>
/// <returns>A <see cref="WaitForChangedResult"/> that contains specific information on the change that occurred.</returns>
public WaitForChangedResult WaitForChanged(WatcherChangeTypes changeType)
public WaitForChangedResult WaitForChanged(WatcherChangeTypes changeType) // NOSONAR
{
throw new NotImplementedException();
}
Expand All @@ -282,7 +274,7 @@ public WaitForChangedResult WaitForChanged(WatcherChangeTypes changeType)
/// <param name="changeType">The System.IO.WatcherChangeTypes to watch for.</param>
/// <param name="timeout">The time (in milliseconds) to wait before timing out.</param>
/// <returns>A <see cref="System.IO.WaitForChangedResult"/> that contains specific information on the change that occurred.</returns>
public WaitForChangedResult WaitForChanged(WatcherChangeTypes changeType, int timeout)
public WaitForChangedResult WaitForChanged(WatcherChangeTypes changeType, int timeout) // NOSONAR
{
throw new NotImplementedException();
}
Expand Down Expand Up @@ -397,7 +389,7 @@ private void RemoveDuplicateEvents(DelayedEvent current, int i)
}
}

private bool ShouldRaiseEvent(DelayedEvent current)
private static bool ShouldRaiseEvent(DelayedEvent current)
{
if (current.Args.ChangeType is WatcherChangeTypes.Created or WatcherChangeTypes.Changed)
{
Expand All @@ -415,7 +407,7 @@ private bool ShouldRaiseEvent(DelayedEvent current)
return true;
}

private void DelayEvent(DelayedEvent? current)
private static void DelayEvent(DelayedEvent? current)
{
if (current != null)
{
Expand Down Expand Up @@ -461,10 +453,29 @@ private void RaiseEvents(Queue? queue)
case WatcherChangeTypes.All:
break;
default:
throw new ArgumentOutOfRangeException();
throw new ArgumentOutOfRangeException($"{dequeuedInvent.Args.ChangeType} is not defined in {nameof(WatcherChangeTypes)}.");
}
}
}

#endregion

protected virtual void Dispose(bool disposing)
{
if (!_isDisposing)
{
if (disposing)
{
_fileSystemWatcher.Dispose();
_timer.Dispose();
}

_isDisposing = true;
}
}

public void Dispose()
{
Dispose(disposing: true);
GC.SuppressFinalize(this);
}
}
1 change: 0 additions & 1 deletion Axuno.Tools/FileSystem/EmbeddedResourceQuery.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ public IEnumerable<string> GetAllResourceNames()
{
foreach (var assembly in _assemblyNames.Keys)
{
var x = assembly.GetFiles();
var resourceNames = GetResourceNames(assembly);
foreach (var resourceName in resourceNames)
{
Expand Down
31 changes: 24 additions & 7 deletions Axuno.Tools/GeoSpatial/Angle.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace Axuno.Tools.GeoSpatial;
/// <summary>
/// Stores an angle and allows conversion to different formats.
/// </summary>
public class Angle : IComparable<Angle>, IEquatable<Angle>, IFormattable
public class Angle : IComparable<Angle>, IEquatable<Angle>, IFormattable, IEqualityComparer<Angle>
{
/// <summary>
/// Initializes a new instance of the Angle class.
Expand Down Expand Up @@ -471,14 +471,11 @@ internal static Tuple<string, int> ParseFormatString(string format)
protected static void ValidateRange(string parameter, double value, double min, double max)
{
// Need to check for double.NaN, double.PositiveInfinity and
// double.NegativeInfinity, which all have strange behavoir with
// double.NegativeInfinity, which all have strange behavior with
// the normal comparison operators.
if (!double.IsNaN(value) && !double.IsInfinity(value))
if (!double.IsNaN(value) && !double.IsInfinity(value) && min <= value && (value <= max))
{
if (min <= value && (value <= max))
{
return; // Don't throw
}
return; // Don't throw
}

var message = string.Format(
Expand Down Expand Up @@ -509,4 +506,24 @@ private bool IsDifferentDerivedClass(Angle? angle)
// Both the types are derived, return true if they are different
return angleType != type;
}

public bool Equals(Angle? x, Angle? y)
{
if (ReferenceEquals(x, y))
{
return true;
}

if (ReferenceEquals(x, null) || ReferenceEquals(y, null))
{
return false;
}

return x.Radians.Equals(y.Radians);
}

public int GetHashCode(Angle obj)
{
return obj.Radians.GetHashCode();
}
}
11 changes: 5 additions & 6 deletions Axuno.Tools/GeoSpatial/GoogleGeo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Axuno.Tools.GeoSpatial;
public class GoogleGeo
{
/// <summary>
/// The location type returned by the Google geo coding API.
/// The location type returned by the Google geocoding API.
/// </summary>
public enum LocationType
{
Expand Down Expand Up @@ -154,11 +154,10 @@ private static async Task<GeoResponse> EvaluateResponse(HttpResponseMessage http
geoResponse.Found = false;
geoResponse.Success = true;
return geoResponse;
case "INVALID_REQUEST":
case "REQUEST_DENIED":
case "OVER_DAILY_LIMIT":
case "OVER_QUERY_LIMIT":
case "UNKNOWN_ERROR":
/*
* INVALID_REQUEST, REQUEST_DENIED, OVER_DAILY_LIMIT, OVER_QUERY_LIMIT, UNKNOWN_ERROR
* are handled by default
*/
default:
geoResponse.Success = false;
geoResponse.Found = false;
Expand Down
Loading

0 comments on commit 4d34af8

Please sign in to comment.