List Sorting and pattern-matching

Question

I'm trying to sort a list of telegramms to a List of Slaves.

If the PrimeAddress and the SecondaryAddress match, the telegrams belongs to the Slave.

The devices are stored in a Datatable.

I want to check if the deivce already contains the telegramm.

My first attempt looks something like this:

public static DataTable mdlform_NewMBUStele(int LoggerID, List<MbusTelegram> mList, DataTable _deviceDataTable)
    {
        //TODO Das ist total dirty und gar nicht clean hier...
        foreach (DataRow dRow in _deviceDataTable.Rows)
        {
            if (dRow.ItemArray[3] is Slave)
            {
                foreach (MbusTelegram mb in mList)
                {
                    int primeID = (int)dRow.ItemArray[1];
                    if (primeID == LoggerID)
                    {
                        Slave slv = (Slave)dRow.ItemArray[3];
                        foreach (MbusTelegram mbus in mList)
                        {
                            if (slv.PrimeAddress == mbus.Header.PrimeAddress && slv.SecondaryAdd == mbus.FixedDataHeader.SecondaryAddress)
                            {
                                if (slv.ListOfTelegramms == null)
                                {
                                    slv.ListOfTelegramms = new List<MbusTelegram>();
                                }
                                if (!slv.ListOfTelegramms.Contains(mbus))
                                {
                                    slv.ListOfTelegramms.Add(mbus);
                                    //TODO Check if the slave already contains the telegramm, if so don't add it..
                                }
                            }
                        }
                    }
                }
            }
        }
        return _deviceDataTable;
    }

Structure of the datatable:

private void IniDataTable()
    {
        _deviceDataTable = new DataTable("Table");
        _deviceDataTable.Columns.Add("ID", typeof(int));
        _deviceDataTable.Columns.Add("IDParent", typeof(int));
        _deviceDataTable.Columns.Add("Name", typeof(string));
        _deviceDataTable.Columns.Add("Object", typeof(object));
        _deviceDataTable.Rows.Add(new object[] { 0, DBNull.Value, "Addressen", null });
        //GenerateDummyDataTable();
        IniDeviceTreeView();
    }

This code doesn't work very well and it doesn't check if the device already contains the telegramm. Any better ideas?

Answer 1

There's a few things you can do to optimize your code. Firstly take all the things that doesn't change out of the inner loop. E.g. every type you use ItemArray. They are not changing for each iteration of the inner loop but with each iteration of the outer loop secondly you seem to be iterating twice over the same collection never using the variable of the outer loop (mb) so you can eleminate that loop entirely. I've done both in the code below. I've also converted the inner most loop to LINQ but that's mostly because I find it easier to read. I'm not sure the Distinct solves the TODO. I'm not sure I get the TODO at all. It might and it might not solve it you'll need to verify that

public static DataTable mdlform_NewMBUStele(int LoggerID, List<MbusTelegram> mList, DataTable _deviceDataTable){
            //TODO Das ist total dirty und gar nicht clean hier...
            foreach (DataRow dRow in _deviceDataTable.Rows.Cast<DataRow>().Where(d=>d.ItemArray[3] is Slave)){
                    var primeID = (int) dRow.ItemArray[1];
                    var slv = (Slave) dRow.ItemArray[3];
                    if (slv.ListOfTelegramms == null){
                        slv.ListOfTelegramms = new List<MbusTelegram>();
                    }
                    var list = slv.ListOfTelegramms;
                    if (primeID == LoggerID){
                        var items = from m in mList
                                    where
                                        slv.PrimeAddress == m.Header.PrimeAddress &&
                                        slv.SecondaryAdd == m.FixedDataHeader.SecondaryAddress && !list.Contains(m, MbusTelegramEqualityComparer.Default)
                                    select m;
                        list.AddRange(items.Distinct(MbusTelegramEqualityComparer.Default));
                        }
                    }
            return _deviceDataTable;
        }

if you also want to LINQify it for readability you could do as below (this might perform slightly worse):

public static DataTable mdlform_NewMBUStele2(int LoggerID, List<MbusTelegram> mList, DataTable _deviceDataTable){
            var pairs = from dRow in _deviceDataTable.Rows.Cast<DataRow>()
                        where dRow.ItemArray[3] is Slave
                        let primeID = (int) dRow.ItemArray[1]
                        let slv = (Slave) dRow.ItemArray[3]
                        let list = slv.ListOfTelegramms
                        where primeID == LoggerID
                        select
                            new{
                                   list,
                                   items = (from m in mList
                                            where
                                                slv.PrimeAddress == m.Header.PrimeAddress &&
                                                slv.SecondaryAdd == m.FixedDataHeader.SecondaryAddress &&
                                                !list.Contains(m, MbusTelegramEqualityComparer.Default)
                                            select m).Distinct(MbusTelegramEqualityComparer.Default)
                               };
            foreach (var pair in pairs){
                pair.list.AddRange(pair.items);
            }
            return _deviceDataTable;
        }

the latter exampl requires that ListOfTelegrams is initialized to an empty list instead of null

EDIT to have value comparison use a IEqualityComparer. The below uses the timestamps only for equality. The code is updated with the usage. If the ListOfTelegrams can contain duplicates you'll need to handle this in the call to distinct as well otherwise you can leave the call to Distinct out.

public class MbusTelegramEqualityComparer : IEqualityComparer<MbusTelegram>{
        public static readonly MbusTelegramEqualityComparer Default = new MbusTelegramEqualityComparer();
        public bool Equals(MbusTelegram x, MbusTelegram y){
            return x.TimeStamp == y.TimeStamp;
        }

        public int GetHashCode(MbusTelegram obj){
            return obj.TimeStamp.GetHashCode();
        }
    }