I have a linq query that returns the count of several ring sizes. However even though it is reusablable to an extent i want to see if i can improve it.

    private async Task<RingSizeLettersDto> CountOfLettersByRingFingerAsync(Func<RingSize, string> selector)
    {
        var ringSizes = await _ringSizeRepository.Get();

        RingSizeLettersDto ringSizeLetters = new RingSizeLettersDto();

        ringSizeLetters.G = ringSizes.Where(x => selector(x) == "G").Count();
        ringSizeLetters.H = ringSizes.Where(x => selector(x) == "H").Count();
        ringSizeLetters.I = ringSizes.Where(x => selector(x) == "I").Count();
        ringSizeLetters.J = ringSizes.Where(x => selector(x) == "J").Count();
        ringSizeLetters.K = ringSizes.Where(x => selector(x) == "K").Count();
        ringSizeLetters.L = ringSizes.Where(x => selector(x) == "L").Count();
        ringSizeLetters.M = ringSizes.Where(x => selector(x) == "M").Count();
        ringSizeLetters.N = ringSizes.Where(x => selector(x) == "N").Count();
        ringSizeLetters.O = ringSizes.Where(x => selector(x) == "O").Count();
        ringSizeLetters.P = ringSizes.Where(x => selector(x) == "P").Count();
        ringSizeLetters.Q = ringSizes.Where(x => selector(x) == "Q").Count();
        ringSizeLetters.R = ringSizes.Where(x => selector(x) == "R").Count();
        ringSizeLetters.S = ringSizes.Where(x => selector(x) == "S").Count();
        ringSizeLetters.T = ringSizes.Where(x => selector(x) == "T").Count();
        ringSizeLetters.U = ringSizes.Where(x => selector(x) == "U").Count();
        ringSizeLetters.V = ringSizes.Where(x => selector(x) == "V").Count();
        ringSizeLetters.W = ringSizes.Where(x => selector(x) == "W").Count();
        ringSizeLetters.X = ringSizes.Where(x => selector(x) == "X").Count();
        ringSizeLetters.Y = ringSizes.Where(x => selector(x) == "Y").Count();
        ringSizeLetters.Z = ringSizes.Where(x => selector(x) == "Z").Count();
        ringSizeLetters.Z1 = ringSizes.Where(x => selector(x) == "Z1").Count();
        ringSizeLetters.Z2 = ringSizes.Where(x => selector(x) == "Z2").Count();
        ringSizeLetters.Z3 = ringSizes.Where(x => selector(x) == "Z3").Count();
        ringSizeLetters.Z4 = ringSizes.Where(x => selector(x) == "Z4").Count();
        ringSizeLetters.Z5 = ringSizes.Where(x => selector(x) == "Z5").Count();
        ringSizeLetters.Z6 = ringSizes.Where(x => selector(x) == "Z6").Count();
        ringSizeLetters.NA = ringSizes.Where(x => selector(x) == "N/A").Count();

        return ringSizeLetters;

    }

CodePudding user response：

I have two improvements in mind:

• You can use Count(x => selector(x) == "G"), which will reduce 1 method invocation per line.

• You can create a Dictionary<string, int> (probably char as key can be more accurate) which represents the count by character, then elsewhere in you application you can access it using the character you want to search for as a key:

var ringSizes = new[] { 1, 2, 2 };
var ringSizeLetters = ringSizes.GroupBy(x => Selector(x))
     .ToDictionary(x => x.Key, v => v.Count());

var ocurrences = ringSizeLetters["A"];

CodePudding user response：

You can keep all your properties if you really want them:

class RingSizeDto{

  public int G { get; set;}
  public int H { get; set;}

... and avoid reflection by backing them with e.g. a dictionary instead:

class RingSizeDto{

  private Dictionary<string, int> _ringSizes = new();

  public int G { get => _ringSizes[nameof(G)]; set _ringSizes[nameof(G)] = value; }
  public int H { get => _ringSizes[nameof(H)]; set _ringSizes[nameof(H)] = value; }
  ...

This means the sizes can be set all in one go by providing the dictionary:

  public RingSizeDto(Dictionary <string, int> sizes){
    _ringSizes = sizes;
  }

And the dictionary can be generated by query e.g.

var sizes = someSizeList.GroupBy(s => s.SizeLetter).ToDictionary(g => g.Key, g => g.Count());

new RingSizeDto(sizes);

Note; if your list doesn't have every possible size, you'll want to avoid a crash on the dictionary access by using TryGetValue instead

public int G { get => _ringSizes.TryGetValue(nameof(G), out var x) ? x : 0; set _ringSizes[nameof(G)] = value; }

CodePudding user response：

Depending on whether your specific use case allows for this, a cleaner solution would be to use a Dictionary. Judging from your code the best type would be IDictionary<string, int>. In this, the string key is the identifier (such as 'Z1') and the integer value is the result of .Count().

I've coughed up a possible code sample below;

public static IDictionary<string, int> GetRingSizeLettersAsDict(
    IEnumerable<string> sizeLetters,
    IEnumerable<object> ringSizes)
{
    IDictionary<string, int> ringSizeLetters = new();

    sizeLetters.Select(
        sl => ringSizeLetters.Add(
            sl,
            ringSizes.Where(rs => selector(rs).Equals(sl)).Count())
    );

    return ringSizeLetters;
}

This is just one of many solutions, but it should do the same as your code example, provided it fits your use case.

CodePudding user response：

This is type of speudo-code but something like this should do the trick..

It is done via System.Reflection

                PropertyInfo[] properties = ringSizeLetters.GetProperties();
                foreach (PropertyInfo property in properties)
                {
                   var propertyName = property.Name;
                   var ringsizeValue = ringSizes.SingleOrDefault(x=> x.PROPERTYNAME == propertyName)
    
                   // if it finds ringsize Value 
                  if(ringsizeValue != null)
                  {
                     property.SetValue(ringSizeLetters, ringsizeValue.Count());
                  }
    
                }