C#•6mo ago

✅ 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

TOKYODRIFT!•6mo 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•6mo ago

uhhh what's the difference?

Pobiega•6mo ago

Yeah just pass in a selector func for operator/subject type Otherwise they seem near identical to me

PixxelKick•6mo 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"

TOKYODRIFT!•6mo ago

thanks for advices

Gaming

Programming

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