C
C#12mo ago
SWEETPONY

✅ Is it possible to refactor this methods and create only one?

I have following methods:
static private HashSet<string> GetSubjectsAddresses(
string channelName,
IEnumerable<SubjectDto> subjects,
IReadOnlyDictionary<string, ChannelSettingsDto> channelSettings)
{
var subjectTypeSettings = channelSettings[channelName].SubjectTypeSettings;

var addresses = new HashSet<string>();

foreach (var subject in subjects)
{
if (!subjectTypeSettings.TryGetValue(subject.Type.Id, out var value)) continue;

if (subject.CustomFields.FieldValues.TryGetValue(value, out var to))
{
var address = to.ToString();

if (address.IsNotNullOrEmpty())
addresses.Add(address);
}
}

return addresses;
}
static private HashSet<string> GetSubjectsAddresses(
string channelName,
IEnumerable<SubjectDto> subjects,
IReadOnlyDictionary<string, ChannelSettingsDto> channelSettings)
{
var subjectTypeSettings = channelSettings[channelName].SubjectTypeSettings;

var addresses = new HashSet<string>();

foreach (var subject in subjects)
{
if (!subjectTypeSettings.TryGetValue(subject.Type.Id, out var value)) continue;

if (subject.CustomFields.FieldValues.TryGetValue(value, out var to))
{
var address = to.ToString();

if (address.IsNotNullOrEmpty())
addresses.Add(address);
}
}

return addresses;
}
5 Replies
SWEETPONY
SWEETPONYOP12mo ago
static private HashSet<string> GetOperatorAddresses(
string channelName,
IEnumerable<SubjectDto> subjects,
IReadOnlyDictionary<string, ChannelSettingsDto> channelSettings)
{
var operatorSourceCustomFieldId = channelSettings[channelName].OperatorSourceCustomFieldId;

var addresses = new HashSet<string>();

foreach (var subject in subjects)
{
if (subject.CustomFields.FieldValues.TryGetValue(operatorSourceCustomFieldId, out var to))
{
var address = to.ToString();

if (address.IsNotNullOrEmpty())
addresses.Add(address);
}
}

return addresses;
}
static private HashSet<string> GetOperatorAddresses(
string channelName,
IEnumerable<SubjectDto> subjects,
IReadOnlyDictionary<string, ChannelSettingsDto> channelSettings)
{
var operatorSourceCustomFieldId = channelSettings[channelName].OperatorSourceCustomFieldId;

var addresses = new HashSet<string>();

foreach (var subject in subjects)
{
if (subject.CustomFields.FieldValues.TryGetValue(operatorSourceCustomFieldId, out var to))
{
var address = to.ToString();

if (address.IsNotNullOrEmpty())
addresses.Add(address);
}
}

return addresses;
}
JakenVeina
JakenVeina12mo ago
uhhh what's the difference?
Pobiega
Pobiega12mo ago
Yeah just pass in a selector func for operator/subject type Otherwise they seem near identical to me
PixxelKick
PixxelKick12mo ago
var subjectTypeSettings = channelSettings[channelName].SubjectTypeSettings;
var subjectTypeSettings = channelSettings[channelName].SubjectTypeSettings;
This line could be refactored up to be a parameter instead of channelName and channelSettings Then make 2 wrapping extension methods that convert from channelName + channelSettings into the fieldId and hand off to that "core" method I recommend avoiding method names like this: IsNotNullOrEmpty. I'd do this instead:
if (address.IsNullOrEmpty())
{
continue;
}
addresses.Add(address);
if (address.IsNullOrEmpty())
{
continue;
}
addresses.Add(address);
"Not" in a method or property is always a code smell as it can create double negatives in your code, making it hard to parse "IsSomething" is always easier to parse and grok than "IsNotSomething"
SWEETPONY
SWEETPONYOP12mo ago
thanks for advices
Want results from more Discord servers?
Add your server