6
\$\begingroup\$

I create a data collection system that has a tree-like structure built on the similarity to the pattern of the factory, and I have the difficulty in working with this structure, more stingrays lot of code to find the element opredelnie.

    public interface ITag : IRegister
{
    string Name { get; set; }
    string SystemName { get; }
}

public interface ISignal : IRegister
{
}

public interface IRegister
{
    Type GetType { get; }
}

public interface IGroup : IRegister
{
    string Name { get; set; }
    string SystemName { get; }
}

public interface IDevice : IRegister
{
    string Name { get; set; }
    string SystemName { get; }
}

public interface IServer : IRegister
{
}

public interface INode : IRegister
{
    string Name { get; set; }
    string SystemName { get; }
}

[Serializable]
public class Server : IServer
{
    public string Name;
    public string SystemName;
    public List<INode> Nodes;


    public Server(string name, List<INode> node)
    {
        Name = name;
        Nodes = node;
        SystemName = "Server";
    }
    public Type GetType
    {
        get { return typeof(Server); }
    }
}

[Serializable]
public class TCP : INode
{
    public string IPAddress;
    public int Port;
    public List<IDevice> Nodes;
    public string Name { get; set; }
    public string SystemName { get; }

    public TCP(string name, string ip, int port, List<IDevice> devices)
    {
        Name = name;
        IPAddress = ip;
        Port = port;
        Nodes = devices;
        SystemName = "Node";
    }

    public TCP()
    {
    }

    public Type GetType
    {
        get { return typeof(TCP); }
    }
}

[Serializable]
public class RTU : INode
{
    public string Name { get; set; }
    public string SystemName { get; }
    public string Port;
    public int SpeedRate;
    public int DataBits;
    public int StopBits;
    public Parity Parity;
    public List<IDevice> Devices;

    public RTU(string name, int sr, int db, int sb, Parity par, string port, List<IDevice> devices)
    {
        Name = name;
        Port = port;
        SpeedRate = sr;
        StopBits = sb;
        DataBits = db;
        Devices = devices;
        Parity = par;
        SystemName = "Node";
    }

    public Type GetType
    {
        get { return typeof(RTU); }
    }
}

[Serializable]
public class Device : IDevice
{
    public int Address;
    public string Name { get; set; }
    public string SystemName { get; }
    public List<IGroup> Groups;

    public Device(int address, string name, List<IGroup> groups)
    {
        Address = address;
        Groups = groups;
        Name = name;
        SystemName = "Device";
    }

    public Type GetType
    {
        get { return typeof(Device); }
    }

    public Type DeviceType(List<IRegister> list )
    {
        return list.GetType();
    }
}

[Serializable]
public class Group : IGroup
{
    public List<ITag> Tags;
    public string Name { get; set; }
    public string SystemName { get; }

    public Group(string name, List<ITag> tags)
    {
        Tags = tags;
        Name = name;
        SystemName = "Group";
    }

    public Type GetType
    {
        get { return typeof(Group); ; }
    }

}

[Serializable]
public class Tag : ITag, IGroup
{
    public ISignal Signal;
    public TypeData Data;
    public TypeModbus TypeModbus;
    //public object Value;
    //public DateTime Time;
    public string Name { get; set; }
    public string SystemName { get; }

    public Tag(ISignal signal, TypeData data, TypeModbus typeModbus,string n, object value = null)
    {
        Signal = signal;
        Data = data;
        TypeModbus = typeModbus;
        //Value = value;
        //Time = time;
        Name = n;
        SystemName = "Tag";
    }


    public Type GetType
    {
        get { return typeof(Tag); }
    }

}

[Serializable]
public class Analog : ISignal
{
    public int Address;
    public int Address_validaty;
    public float MinWarning;
    public float MinEmergency;
    public float MaxWarning;
    public float MaxEmergency;
    public bool Control;
    public float Coeficient;
    public float Shift;
    public bool IsCoeficient;
    public string MinWText;
    public string MinEText;
    public string MaxWText;
    public string MaxEText;

    public Type GetType
    {
        get { return typeof(Analog); }
    }
}

[Serializable]
public class Discrete : ISignal
{
    public int Address;
    public int Address_validaty;
    public bool IsAutomat;
    public static ITag Tag = null;
    public string TrueText;
    public string FalseText;
    public Discrete(int ad, int adv,  bool isautomat, string ft, string tt, ITag tag = null)
    {
        Address = ad;
        Address_validaty = adv;
        TrueText = tt;
        FalseText = ft;
        IsAutomat = isautomat;

        if (isautomat)
            Tag = tag;
    }
    public Type GetType
    {
        get { return typeof(Discrete); }
    }

}

[Serializable]
public class Managment : ISignal
{
    public ITag ConnectionRegister;
    public int Address;
    public int SecondsReply;


    public Type GetType
    {
        get { return typeof(Managment); }
    }
}

In this example I'm using nested loops to find Tag on List which contains the entire structure, and are searching for me to have to use for each node of the structure, and it only renames the item, and there is deletion, search duplicate, overlap, and so on.

    if(e.Node.Name.Equals("Tag"))
{
    foreach(Server z in List)
    {
        foreach(INode node in z.Nodes)
        {
            if(node.GetType == typeof(TCP))
            {
                TCP _node = (TCP) node;
                foreach(Device device in _node.Devices)
                {
                    IEnumerable<IGroup> d = device.Groups.Where(p => p.GetType == typeof(Group));
                    foreach(Group group in d)
                    {
                        var n = group.Tags.Where(p => p.Name == e.Node.Text);
                        foreach(ITag tag in n)
                        {
                            tag.Name = newname;
                        }
                    }
                }
            }
            else if(node.GetType == typeof(RTU))
            {
                RTU _node = (RTU) node;
                foreach(Device device in _node.Devices)
                {
                    IEnumerable<IGroup> d = device.Groups.Where(p => p.GetType == typeof(Group));
                    foreach(Group group in d)
                    {
                        var n = group.Tags.Where(p => p.Name == e.Node.Text);
                        foreach(ITag tag in n)
                        {
                            tag.Name = newname;
                        }
                    }
                }
            }
        }
    }
}

How can I simplify the search, make it more readable?

\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

If you extend the INode interface to also has a property returning a List<IDevice> like so

public interface INode : IRegister
{
    string Name { get; set; }
    string SystemName { get; }
    List<IDevice> Devices { get; }
}  

you could remove a lot of code duplication because you wouldn't need to distinguish between TCP and RTU.

By adding a method GetTags() like so

private IEnumerable<ITag> GetTags(Server server, string tagName, Type groupType)
{
    foreach (INode node in server.Nodes)
    {
        foreach (IDevice device in node.Devices)
        {
            foreach (IGroup group in device.Groups.Where(g => g.GetType() == groupType))
            {
                foreach (ITag tag in group.Tags.Where(t => t.Name == tagName))
                {
                    yield return tag;
                }
            }
        }
    }
}  

your former loop could look like so

Type groupType = typeof(Group);
String tagName = e.Node.Text;

foreach(Server server in List)
{
    foreach(ITag tag in GetTags(server, tagName, groupType))
    {
        tag.Name = newname;
    }  
}  

you see I have used the GetType() method of the object class. The provided GetType property of the IRegister interface will raise a lot of warnings, because it hides the inherited member objectGetType().

\$\endgroup\$
4
\$\begingroup\$

You call following code:

TCP _node = (TCP) node;
foreach(Device device in _node.Devices)
{
    //...
}

Yet, I don't see a property Devices in your TCP class, it's called Nodes there.

That on the side, it means you perform a foreach loop on the List<IDevice> of either TCP or RTU. You can extract that logic in a separate method and call it. Here's the extracted method:

void SetTagNames(IList<Device> devices)
{
    foreach(Device device in devices)
    {
        IEnumerable<IGroup> d = device.Groups.Where(p => p.GetType == typeof(Group));
        foreach(Group group in d)
        {
            var n = group.Tags.Where(p => p.Name == e.Node.Text);
            foreach(ITag tag in n)
            {
                tag.Name = newname;
            }
        }
    }
}

And your calling code would become:

//...
if (node.GetType == typeof(TCP))
{
    SetTagNames(((TCP)node).Devices);
}
else if (node.GetType == typeof(RTU))
{
    SetTagNames(((RTU)node).Devices);
}
//...
\$\endgroup\$
4
  • \$\begingroup\$ @Heslacher Thanks, great review of you too! You often have a different view than I have on the same matters and I like that, I learn a lot reading your reviews. :) \$\endgroup\$
    – Abbas
    Commented Sep 30, 2015 at 9:01
  • 1
    \$\begingroup\$ @Heslacher big thanks for previous comment, corrected my code! \$\endgroup\$
    – Abbas
    Commented Sep 30, 2015 at 9:07
  • \$\begingroup\$ oh, here are many nested loops again \$\endgroup\$ Commented Sep 30, 2015 at 11:09
  • 2
    \$\begingroup\$ @MaxMelnikov No matter how you rewrite your code, you're not going to avoid looping. \$\endgroup\$
    – Abbas
    Commented Sep 30, 2015 at 11:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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