3
\$\begingroup\$

I am writing a C# program wherein I need to populate an array based on a lookup table and set of string arrays with metadata. My lookup table looks like this (Table with key: transmitter, value: Array of receiver):

{
    LED1: ["px1","px2","px3"],
    LED2: ["px4","px5","px6"]
}

and my meta arrays looks like this (it is dynamic (just an example) and comes as a response from a DB query):

var transmitters = new string[] { "LED1", "LED2" };
var receivers = new string[] { "px1", "px2", "px3", "px4", "px5", "px6" };

My requirement is:

  • If the transmitter LED1 or LED2 (or any other transmitter) is present in the lookup table, the value of the transmitter (i.e. ["px1","px2","px3"]) has to be compared with the receiver which are present in the lookup and led has to be marked yellow.
  • Orphan transmitter or/ receiver has to be marked red.

Example

Lookup

{
        LED1: ["px1", "px2", "px3"],
        LED2: ["px5", "px8"]
}

Transmitters and receivers

var transmitters = new string[] { "led1", "led2" };
var receivers = new string[] { "px1", "px2", "px3", "px4", "px5", "px6" };

The result should be a list as:

led1-yellow
px1-yellow
px2-yellow
px3-yellow
led2-yellow
px5-yellow
px4-red
px6-red.

I have written code that works:

using System;
using System.Collections.Generic;
using System.Linq;


public class Program
{
    public static void Main()
    {
      var transmitters = new string[] { "led1", "led2", "led3" };
      var receivers = new string[] { "px1", "px2", "px3", "px4", "px5", "px6" };
      var lookup = new Dictionary<string, string[]>() {
          { "led1", new string[] { "px1", "px2", "px3" } },
          { "led2", new string[] { "px5", "px8"} }
      };

      var blocks = new List<Block>();
      var blocksTracker = new List<string>();

      foreach (var transmitter in transmitters)
      {
        if (lookup.ContainsKey(transmitter))
        {
          var receiverLookup = lookup[transmitter];
          var intersection = receivers.Intersect(receiverLookup).ToArray();
          if (intersection.Length > 0)
          {
             blocks.Add(new Block() { Id = transmitter, status = "yellow"});
             blocksTracker.Add(transmitter);
            foreach (var receiver in intersection)
            {
              blocks.Add(new Block(){Id = receiver, status = "yellow"});
              blocksTracker.Add(receiver);
            }
          } else  
          {
            blocks.Add(new Block(){Id = transmitter, status = "red"});
            blocksTracker.Add(transmitter);   
          }
        }
      }

      var ungrouped = receivers.Except(blocksTracker).ToArray();

      foreach (var receiver in ungrouped)
      {
        blocks.Add(new Block(){Id = receiver, status = "red"});
        blocksTracker.Add(receiver);
      }

      foreach (var i in blocks)
      {
        Console.WriteLine(i.Id + "-"+i.status);
      }
    }

    public class Block
    {
      public string Id { get; set; }

      public string status { get; set; }
    }
}

I am new to C# and I wanted to know if there is a better way of doing this. You can see the working Fiddle here.

\$\endgroup\$
  • \$\begingroup\$ I have updated the question. Please have a look. Also you can see it working dotnetfiddle.net/d3E7n0 \$\endgroup\$ – vikk Apr 26 at 11:19
7
\$\begingroup\$
        if (lookup.ContainsKey(transmitter))
        {
          var receiverLookup = lookup[transmitter];

This searches for the KeyValuePair twice. There's a more efficient approach:

        if (lookup.TryGetValue(transmitter, out var receiverLookup))

      var ungrouped = receivers.Except(blocksTracker).ToArray();

      foreach (var receiver in ungrouped)
      {
        blocks.Add(new Block(){Id = receiver, status = "red"});
        blocksTracker.Add(receiver);
      }

The ToArray() there is unnecessary: the enumerable can be left as a lazy enumerable because the only use is to iterate over it once.


          var intersection = receivers.Intersect(receiverLookup).ToArray();
          if (intersection.Length > 0)
          {
             blocks.Add(new Block() { Id = transmitter, status = "yellow"});
             blocksTracker.Add(transmitter);
            foreach (var receiver in intersection)
            {
              blocks.Add(new Block(){Id = receiver, status = "yellow"});
              blocksTracker.Add(receiver);
            }
          } else  
          {
            blocks.Add(new Block(){Id = transmitter, status = "red"});
            blocksTracker.Add(transmitter);   
          }

This seems rather complicated. I think the whole thing could be simplified:

var transmittersPaired = new HashSet<string>();
var receiversPaired = new HashSet<string>();

foreach (var transmitter in transmitters)
{
    if (lookup.TryGetValue(transmitter, out var receiverLookup) && receiverLookup.Any())
    {
        transmittersPaired.Add(transmitter);
        foreach (var receiver in receiverLookup)
        {
            receiversSeen.Add(receiver);
        }
    }
}

var blocks = new List<Block>();
foreach (var transmitter in transmitters)
{
    blocks.Add(new Block { Id = transmitter, status = transmittersPaired.Contains(transmitter) ? "yellow" : "red" });
}
foreach (var receiver in receivers)
{
    blocks.Add(new Block { Id = receiver, status = receiversPaired.Contains(receiver) ? "yellow" : "red" });
}

There's still some repeated code, which might be simplified in one of two ways. If there's a guarantee that the transmitters and receivers will never share IDs then transmittersPaired and receiversPaired could be merged into one set, and the foreach loops at the end could be merged into one loop over transmitters.Concat(receivers). Alternatively, a method could be factored out.

\$\endgroup\$
  • \$\begingroup\$ Makes sense ...Thanks :) \$\endgroup\$ – vikk Apr 29 at 4:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.