Skip to content

Commit 9e56cf0

Browse files
committed
Addressed review comments
- Refactor file opening methods to use Type inputs instead of generics, allowing for a more flexible calling pattern - Define an abstract Clone() method for ProbeGroups instead of using reflection to construct copied classes - Brought Rhs2116 dialogs up to date with other dialogs, there were issues discovered during testing related to loading/saving files
1 parent deba9a4 commit 9e56cf0

11 files changed

+59
-46
lines changed

OpenEphys.Onix1.Design/ChannelConfigurationDialog.cs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -278,15 +278,16 @@ private void FormShown(object sender, EventArgs e)
278278
if (!TopLevel)
279279
{
280280
menuStrip.Visible = false;
281-
ResizeZedGraph();
282281
}
282+
283+
ResizeZedGraph();
283284
}
284285

285-
internal virtual bool OpenFile<T>() where T : ProbeGroup
286+
internal virtual bool OpenFile(Type type)
286287
{
287-
var newConfiguration = OpenAndParseConfigurationFile<T>();
288+
var newConfiguration = OpenAndParseConfigurationFile(type);
288289

289-
if (ValidateProbeGroup(newConfiguration))
290+
if (newConfiguration != null)
290291
{
291292
ProbeGroup = newConfiguration;
292293
return true;
@@ -317,7 +318,7 @@ internal bool ValidateProbeGroup(ProbeGroup newConfiguration)
317318
}
318319
}
319320

320-
internal T OpenAndParseConfigurationFile<T>() where T : ProbeGroup
321+
internal ProbeGroup OpenAndParseConfigurationFile(Type type)
321322
{
322323
using OpenFileDialog ofd = new();
323324

@@ -328,9 +329,9 @@ internal T OpenAndParseConfigurationFile<T>() where T : ProbeGroup
328329

329330
if (ofd.ShowDialog() == DialogResult.OK && File.Exists(ofd.FileName))
330331
{
331-
var newConfiguration = JsonHelper.DeserializeString<T>(File.ReadAllText(ofd.FileName));
332+
var newConfiguration = ProbeInterfaceHelper.LoadExternalProbeInterfaceFile(ofd.FileName, type);
332333

333-
return newConfiguration;
334+
return ValidateProbeGroup(newConfiguration) ? (ProbeGroup)newConfiguration : null;
334335
}
335336

336337
return null;
@@ -1025,7 +1026,7 @@ void ResizeAxes()
10251026

10261027
private void MenuItemOpenFile(object sender, EventArgs e)
10271028
{
1028-
if (OpenFile<ProbeGroup>())
1029+
if (OpenFile(ProbeGroup.GetType()))
10291030
{
10301031
DrawProbeGroup();
10311032
ResetZoom();

OpenEphys.Onix1.Design/NeuropixelsV1ChannelConfigurationDialog.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,11 @@ internal override void LoadDefaultChannelLayout()
5858
OnFileOpenHandler();
5959
}
6060

61-
internal override bool OpenFile<T>()
61+
internal override bool OpenFile(Type type)
6262
{
63-
if (base.OpenFile<NeuropixelsV1eProbeGroup>())
63+
if (base.OpenFile(type))
6464
{
6565
ProbeConfiguration.ProbeGroup = (NeuropixelsV1eProbeGroup)ProbeGroup;
66-
6766
OnFileOpenHandler();
6867

6968
return true;

OpenEphys.Onix1.Design/NeuropixelsV2eChannelConfigurationDialog.cs

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public partial class NeuropixelsV2eChannelConfigurationDialog : ChannelConfigura
2424
/// </summary>
2525
/// <param name="probeConfiguration">A <see cref="NeuropixelsV2ProbeConfiguration"/> object holding the current configuration settings.</param>
2626
public NeuropixelsV2eChannelConfigurationDialog(NeuropixelsV2ProbeConfiguration probeConfiguration)
27-
: base(probeConfiguration.ProbeGroup)
27+
: base(probeConfiguration.ProbeGroup.Clone())
2828
{
2929
zedGraphChannels.ZoomButtons = MouseButtons.None;
3030
zedGraphChannels.ZoomButtons2 = MouseButtons.None;
@@ -54,23 +54,13 @@ internal override void LoadDefaultChannelLayout()
5454
OnFileOpenHandler();
5555
}
5656

57-
internal override bool OpenFile<T>()
57+
internal override bool OpenFile(Type type)
5858
{
59-
MethodInfo method = GetType().GetMethod(
60-
nameof(OpenAndParseConfigurationFile),
61-
BindingFlags.Instance | BindingFlags.NonPublic,
62-
null,
63-
Type.EmptyTypes,
64-
null) ?? throw new InvalidOperationException($"Could not find the {nameof(OpenAndParseConfigurationFile)} method.");
65-
66-
MethodInfo genericMethod = method.MakeGenericMethod(ProbeGroup.GetType()) ?? throw new InvalidOperationException($"Could not create the generic {nameof(OpenAndParseConfigurationFile)} method.");
67-
68-
var newConfiguration = genericMethod.Invoke(this, null) as NeuropixelsV2eProbeGroup;
69-
70-
if (ValidateProbeGroup(newConfiguration))
59+
if (base.OpenFile(type))
7160
{
72-
ProbeGroup = newConfiguration;
61+
ProbeConfiguration.ProbeGroup = (NeuropixelsV2eProbeGroup)ProbeGroup;
7362
OnFileOpenHandler();
63+
7464
return true;
7565
}
7666

@@ -80,7 +70,6 @@ internal override bool OpenFile<T>()
8070
private void OnFileOpenHandler()
8171
{
8272
OnFileLoad?.Invoke(this, EventArgs.Empty);
83-
ProbeConfiguration.ProbeGroup = (NeuropixelsV2eProbeGroup)ProbeGroup;
8473
}
8574

8675
internal override void ZoomEvent(ZedGraphControl sender, ZoomState oldState, ZoomState newState)

OpenEphys.Onix1.Design/Rhs2116ChannelConfigurationDialog.cs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ public partial class Rhs2116ChannelConfigurationDialog : ChannelConfigurationDia
1313
{
1414
internal event EventHandler OnSelect;
1515
internal event EventHandler OnZoom;
16+
internal event EventHandler OnFileLoad;
1617

1718
/// <summary>
1819
/// Initializes a new instance of <see cref="Rhs2116ChannelConfigurationDialog"/>.
@@ -36,14 +37,33 @@ public Rhs2116ChannelConfigurationDialog(Rhs2116ProbeGroup probeGroup)
3637
RefreshZedGraph();
3738
}
3839

40+
internal override void LoadDefaultChannelLayout()
41+
{
42+
ProbeGroup = DefaultChannelLayout();
43+
44+
OnFileOpenHandler();
45+
}
46+
3947
internal override ProbeGroup DefaultChannelLayout()
4048
{
4149
return new Rhs2116ProbeGroup();
4250
}
4351

44-
internal override bool OpenFile<T>()
52+
internal override bool OpenFile(Type type)
53+
{
54+
if (base.OpenFile(type))
55+
{
56+
OnFileOpenHandler();
57+
58+
return true;
59+
}
60+
61+
return false;
62+
}
63+
64+
void OnFileOpenHandler()
4565
{
46-
return base.OpenFile<Rhs2116ProbeGroup>();
66+
OnFileLoad?.Invoke(this, EventArgs.Empty);
4767
}
4868

4969
internal override void SelectedContactChanged()

OpenEphys.Onix1.Design/Rhs2116StimulusSequenceDialog.cs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,7 @@ public Rhs2116StimulusSequenceDialog(ConfigureRhs2116Trigger rhs2116Trigger)
7171

7272
ChannelDialog.OnSelect += OnSelect;
7373
ChannelDialog.OnZoom += OnZoom;
74-
75-
ChannelDialog.Show();
74+
ChannelDialog.OnFileLoad += OnFileLoad;
7675

7776
StimulusSequenceOptions = new();
7877
groupBoxDefineStimuli.Controls.Add(StimulusSequenceOptions.SetChildFormProperties(this));
@@ -115,6 +114,11 @@ public Rhs2116StimulusSequenceDialog(ConfigureRhs2116Trigger rhs2116Trigger)
115114
DrawStimulusWaveform();
116115
}
117116

117+
void OnFileLoad(object sender, EventArgs e)
118+
{
119+
Trigger.ProbeGroup = (Rhs2116ProbeGroup)ChannelDialog.ProbeGroup;
120+
}
121+
118122
internal void OnZoom(object sender, EventArgs e)
119123
{
120124
ChannelDialog.UpdateFontSize();
@@ -885,9 +889,7 @@ internal override bool IsSequenceValid()
885889

886890
internal override void DeserializeStimulusSequence(string fileName)
887891
{
888-
var newSequence = JsonHelper.DeserializeString<Rhs2116StimulusSequencePair>(File.ReadAllText(fileName));
889-
890-
if (newSequence != null && newSequence.Stimuli.Length == 32)
892+
if (JsonHelper.DeserializeString(File.ReadAllText(fileName), typeof(Rhs2116StimulusSequencePair)) is Rhs2116StimulusSequencePair newSequence && newSequence.Stimuli.Length == 32)
891893
{
892894
if (newSequence == new Rhs2116StimulusSequencePair())
893895
{

OpenEphys.Onix1/JsonHelper.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace OpenEphys.Onix1
77
{
88
internal static class JsonHelper
99
{
10-
public static T DeserializeString<T>(string jsonString) where T : class
10+
public static object DeserializeString(string jsonString, Type type)
1111
{
1212
var errors = new List<string>();
1313

@@ -22,7 +22,7 @@ public static T DeserializeString<T>(string jsonString) where T : class
2222

2323
try
2424
{
25-
var obj = JsonConvert.DeserializeObject<T>(jsonString, serializerSettings);
25+
var obj = JsonConvert.DeserializeObject(jsonString, type, serializerSettings);
2626

2727
if (errors.Count > 0)
2828
{

OpenEphys.Onix1/NeuropixelsV1ProbeConfiguration.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ public string ProbeInterfaceLoadFileName
266266
if (string.IsNullOrEmpty(probeInterfaceFileName))
267267
return new NeuropixelsV1eProbeGroup();
268268

269-
return ProbeInterfaceHelper.LoadExternalProbeInterfaceFile<NeuropixelsV1eProbeGroup>(probeInterfaceFileName);
269+
return ProbeInterfaceHelper.LoadExternalProbeInterfaceFile(probeInterfaceFileName, typeof(NeuropixelsV1eProbeGroup)) as NeuropixelsV1eProbeGroup;
270270
});
271271
}
272272
}

OpenEphys.Onix1/NeuropixelsV2QuadShankProbeConfiguration.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public NeuropixelsV2QuadShankProbeConfiguration(NeuropixelsV2Probe probe, Neurop
7272
public NeuropixelsV2QuadShankProbeConfiguration(NeuropixelsV2QuadShankProbeConfiguration probeConfiguration)
7373
{
7474
Reference = probeConfiguration.Reference;
75-
ProbeGroup = probeConfiguration.ProbeGroup.Clone<NeuropixelsV2eQuadShankProbeGroup>();
75+
ProbeGroup = probeConfiguration.ProbeGroup.Clone();
7676
Probe = probeConfiguration.Probe;
7777
}
7878

@@ -86,7 +86,7 @@ public NeuropixelsV2QuadShankProbeConfiguration(NeuropixelsV2QuadShankProbeConfi
8686
[JsonConstructor]
8787
public NeuropixelsV2QuadShankProbeConfiguration(NeuropixelsV2eQuadShankProbeGroup probeGroup, NeuropixelsV2Probe probe, NeuropixelsV2QuadShankReference reference)
8888
{
89-
ProbeGroup = probeGroup.Clone<NeuropixelsV2eQuadShankProbeGroup>();
89+
ProbeGroup = probeGroup.Clone();
9090
Reference = reference;
9191
Probe = probe;
9292
}

OpenEphys.Onix1/NeuropixelsV2eProbeGroup.cs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,7 @@ public NeuropixelsV2eProbeGroup(NeuropixelsV2eProbeGroup probeGroup)
3131
{
3232
}
3333

34-
internal NeuropixelsV2eProbeGroup Clone<T>() where T : NeuropixelsV2eProbeGroup
35-
{
36-
return Activator.CreateInstance(typeof(T), Specification, Version, Probes.Select(probe => new Probe(probe)).ToArray()) as T ?? throw new NullReferenceException($"Issue creating new probe group of type {nameof(NeuropixelsV2eProbeGroup)}.");
37-
}
34+
internal abstract NeuropixelsV2eProbeGroup Clone();
3835

3936
internal void UpdateDeviceChannelIndices(NeuropixelsV2Electrode[] channelMap)
4037
{

OpenEphys.Onix1/NeuropixelsV2eQuadShankProbeGroup.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ public NeuropixelsV2eQuadShankProbeGroup(NeuropixelsV2eQuadShankProbeGroup probe
7474
{
7575
}
7676

77+
internal override NeuropixelsV2eProbeGroup Clone()
78+
{
79+
return new NeuropixelsV2eQuadShankProbeGroup(Specification, Version, Probes.Select(probe => new Probe(probe)).ToArray());
80+
}
81+
7782
/// <summary>
7883
/// Generates a 2D array of default contact positions based on the given number of channels.
7984
/// </summary>

0 commit comments

Comments
 (0)