Skip to content

Commit a1037c3

Browse files
CopilotBornToBeRoot
andcommitted
Fix remaining code review issues: extract common error handling, add null checks, use timestamped backups
Co-authored-by: BornToBeRoot <[email protected]>
1 parent 47faca2 commit a1037c3

File tree

2 files changed

+31
-28
lines changed

2 files changed

+31
-28
lines changed

Source/NETworkManager.Settings/SettingsManager.cs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using log4net;
22
using NETworkManager.Models;
33
using NETworkManager.Models.Network;
4+
using NETworkManager.Utilities;
45
using System;
56
using System.IO;
67
using System.Linq;
@@ -150,10 +151,10 @@ public static void Load()
150151
// Save in new JSON format
151152
Save();
152153

153-
// Backup the old XML file
154+
// Backup the old XML file with timestamp to avoid overwriting existing backups
154155
var backupFilePath = Path.Combine(GetSettingsFolderLocation(),
155-
$"{SettingsFileName}{LegacySettingsFileExtension}.backup");
156-
File.Copy(legacyFilePath, backupFilePath, true);
156+
$"{SettingsFileName}_{TimestampHelper.GetTimestamp()}{LegacySettingsFileExtension}.backup");
157+
File.Copy(legacyFilePath, backupFilePath, false);
157158
Log.Info($"Legacy XML settings file backed up to: {backupFilePath}");
158159

159160
// Note: The original XML file is intentionally not deleted to allow users to revert if needed.
@@ -198,7 +199,12 @@ private static SettingsInfo DeserializeFromXmlFile(string filePath)
198199

199200
using var fileStream = new FileStream(filePath, FileMode.Open);
200201

201-
var settingsInfo = (SettingsInfo)xmlSerializer.Deserialize(fileStream);
202+
var settingsInfo = xmlSerializer.Deserialize(fileStream) as SettingsInfo;
203+
204+
if (settingsInfo == null)
205+
{
206+
throw new InvalidOperationException("Failed to deserialize settings from XML file. The result was null.");
207+
}
202208

203209
return settingsInfo;
204210
}

Source/NETworkManager/App.xaml.cs

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -96,36 +96,14 @@ by BornToBeRoot
9696
Log.Error("Could not load application settings!");
9797
Log.Error(ex.Message + "-" + ex.StackTrace);
9898

99-
// Create backup of corrupted file
100-
var destinationFile =
101-
$"{TimestampHelper.GetTimestamp()}_corrupted_" + SettingsManager.GetSettingsFileName();
102-
File.Copy(SettingsManager.GetSettingsFilePath(),
103-
Path.Combine(SettingsManager.GetSettingsFolderLocation(), destinationFile));
104-
Log.Info($"A backup of the corrupted settings file has been saved under {destinationFile}");
105-
106-
// Initialize default application settings
107-
Log.Info("Initialize default application settings...");
108-
109-
SettingsManager.Initialize();
110-
ConfigurationManager.Current.ShowSettingsResetNoteOnStartup = true;
99+
HandleCorruptedSettingsFile();
111100
}
112101
catch (JsonException ex)
113102
{
114103
Log.Error("Could not load application settings! JSON file is corrupted or invalid.");
115104
Log.Error(ex.Message + "-" + ex.StackTrace);
116105

117-
// Create backup of corrupted file
118-
var destinationFile =
119-
$"{TimestampHelper.GetTimestamp()}_corrupted_" + SettingsManager.GetSettingsFileName();
120-
File.Copy(SettingsManager.GetSettingsFilePath(),
121-
Path.Combine(SettingsManager.GetSettingsFolderLocation(), destinationFile));
122-
Log.Info($"A backup of the corrupted settings file has been saved under {destinationFile}");
123-
124-
// Initialize default application settings
125-
Log.Info("Initialize default application settings...");
126-
127-
SettingsManager.Initialize();
128-
ConfigurationManager.Current.ShowSettingsResetNoteOnStartup = true;
106+
HandleCorruptedSettingsFile();
129107
}
130108

131109
// Upgrade settings if necessary
@@ -239,6 +217,25 @@ by BornToBeRoot
239217
}
240218
}
241219

220+
/// <summary>
221+
/// Handles a corrupted settings file by creating a backup and initializing default settings.
222+
/// </summary>
223+
private void HandleCorruptedSettingsFile()
224+
{
225+
// Create backup of corrupted file
226+
var destinationFile =
227+
$"{TimestampHelper.GetTimestamp()}_corrupted_" + SettingsManager.GetSettingsFileName();
228+
File.Copy(SettingsManager.GetSettingsFilePath(),
229+
Path.Combine(SettingsManager.GetSettingsFolderLocation(), destinationFile));
230+
Log.Info($"A backup of the corrupted settings file has been saved under {destinationFile}");
231+
232+
// Initialize default application settings
233+
Log.Info("Initialize default application settings...");
234+
235+
SettingsManager.Initialize();
236+
ConfigurationManager.Current.ShowSettingsResetNoteOnStartup = true;
237+
}
238+
242239
private void DispatcherTimer_Tick(object sender, EventArgs e)
243240
{
244241
Log.Info("Run background job...");

0 commit comments

Comments
 (0)