0
\$\begingroup\$

I have a WPF application that gets data from a web-server.

It contains two Views

  • LeftView
  • RightView

three Models

  • LeftModel
  • RightModel
  • CentralModel

and two ViewModels

  • LeftViewModel
  • RightViewModel

I will show only LeftView, LeftViewModel, LeftModel, and CentralModel (too much code). You can find the entire project here.

I guess the main problem is that the UpdateCollection() have high coupling with public ObservableCollection<SomeTypeA> Items {get; set;}.

Therefore I have the feeling that I cannot place UpdateCollection() in CentralModel.

I think it will be better if UpdateCollection() will be in CentralModel

Work logic is very simple, incoming message from web server add into public Dictionary<string, Action<MessageReceivedEventArgs>> Handle { get; set; }

        public void Message(object sender, MessageReceivedEventArgs e)
    {
        var dresult = Newtonsoft.Json.JsonConvert.DeserializeObject<Dictionary<string, SomeTypeA>>(e.Message);
        if (Handle.ContainsKey(dresult.Keys.ToList()[0]))
        {
            Handle[dresult.Keys.ToList()[0]](e);
        }
    }

, if dictionary contains key, it firing event in model

CentralModel.Instance.Handle.Add("central_office", (m) =>
        {
            var dresult = Newtonsoft.Json.JsonConvert.DeserializeObject<Dictionary<string, SomeTypeA>>(m.Message);
            Console.WriteLine(m.Message.ToString());
            foreach (KeyValuePair<string,SomeTypeA> item in dresult)
            {
                if (!Items.Any(key=>key.ID==dresult["central_office"].ID))
                {
                    Application.Current.Dispatcher.Invoke(DispatcherPriority.Normal, new Action(() => Items.Add(item.Value)));
                }
                foreach (SomeTypeA subitem in Items)
                {
                    subitem.ID = item.Value.ID;
                    subitem.Name = item.Value.Name;
                    subitem.Value = item.Value.Value;
                    subitem.Work = item.Value.Work;
                    subitem.Department = item.Value.Department;
                }
            }
        });

ServerClass.cs

namespace Server
{
class ServerClass
{
    private WebSocketServer appServer;

    public void Setup()
    {
        appServer = new WebSocketServer();

        if (!appServer.Setup(2012)) //Setup with listening port
        {
            Console.WriteLine("Failed to setup!");
            Console.ReadKey();
            return;
        }

        appServer.NewMessageReceived += new SessionHandler<WebSocketSession, string>(appServer_NewMessageReceived);

        Console.WriteLine();
    }

    public void Start()
    {
        if (!appServer.Start())
        {
            Console.WriteLine("Failed to start!");
            Console.ReadKey();
            return;
        }

        Console.WriteLine("The server started successfully! Press any key to see application options.");

        SomeTypeA FirstWorker = new SomeTypeA()
        {
            Department = "Finance",
            ID = "0",
            Name = "John",
            Work = "calculate money"
        };
        SomeTypeB SecondWorker = new SomeTypeB()
        {
            ID = "1",
            Name = "Nick",
            Work = "clean toilet"
        };

        while (true)
        {
            FirstWorker.value += 1;
            SecondWorker.value += 5;
            Dictionary<string, SomeTypeA> Element1 = new Dictionary<string, SomeTypeA>();
            Element1.Add("central_office", FirstWorker);
            Dictionary<string, SomeTypeB> Element2 = new Dictionary<string, SomeTypeB>();
            Element2.Add("back_office", SecondWorker);
            string message1 = Newtonsoft.Json.JsonConvert.SerializeObject(Element1);
            string message2 = Newtonsoft.Json.JsonConvert.SerializeObject(Element2);

            System.Threading.Thread.Sleep(2000);
            foreach (WebSocketSession session in appServer.GetAllSessions())
            {
                session.Send(message1);
                session.Send(message2);
            }
        }
    }

    private void appServer_NewMessageReceived(WebSocketSession session, string message)
    {
        Console.WriteLine("Client said: " + message);
        session.Send("Server responded back: " + message);
    }
}
}

Program.cs

namespace Server
{
class Program
{
    static void Main(string[] args)
    {
        ServerClass myServer = new ServerClass();
        myServer.Setup();
        myServer.Start();
    }
}
}

LeftView.cs

<Grid>
    <ListView ItemsSource="{Binding LM.Items}">
        <ListView.ItemTemplate>
            <DataTemplate>
                <Grid>
                    <Grid.ColumnDefinitions>
                        <ColumnDefinition></ColumnDefinition>
                        <ColumnDefinition></ColumnDefinition>
                        <ColumnDefinition></ColumnDefinition>
                        <ColumnDefinition></ColumnDefinition>
                        <ColumnDefinition></ColumnDefinition>
                    </Grid.ColumnDefinitions>
                    <Label Grid.Column="0" Content="{Binding Name}"></Label>
                    <Label Grid.Column="1" Content="{Binding Work}"></Label>
                    <Label Grid.Column="2" Content="{Binding Value}"></Label>
                    <Label Grid.Column="3" Content="{Binding ID}"></Label>
                    <Label Grid.Column="4" Content="{Binding Department}"></Label>
                </Grid>
            </DataTemplate>
        </ListView.ItemTemplate>
    </ListView>
</Grid>

LeftViewModel.cs

namespace WpfApplication139.ViewModels
{
public class LeftViewModel
{
    public LeftModel LM { get; set; }
    public LeftViewModel()
    {
        LM = new LeftModel();
    }
}
}

LeftModel.cs

namespace WpfApplication139.Models
{
public class LeftModel
{
    public ObservableCollection<SomeTypeA> Items {get; set;}
    public LeftModel()
    {
        Items = new ObservableCollection<SomeTypeA>();
        CentralModel.Instance.Setup("ws://127.0.0.1:2012", "basic", WebSocketVersion.Rfc6455);
        CentralModel.Instance.Start();
        UpdateCollection();
    }

    public void UpdateCollection()
    {
        CentralModel.Instance.Handle.Add("central_office", (m) =>
        {
            var dresult = Newtonsoft.Json.JsonConvert.DeserializeObject<Dictionary<string, SomeTypeA>>(m.Message);
            Console.WriteLine(m.Message.ToString());
            foreach (KeyValuePair<string,SomeTypeA> item in dresult)
            {
                if (!Items.Any(key=>key.ID==dresult["central_office"].ID))
                {
                    Application.Current.Dispatcher.Invoke(DispatcherPriority.Normal, new Action(() => Items.Add(item.Value)));
                }
                foreach (SomeTypeA subitem in Items)
                {
                    subitem.ID = item.Value.ID;
                    subitem.Name = item.Value.Name;
                    subitem.Value = item.Value.Value;
                    subitem.Work = item.Value.Work;
                    subitem.Department = item.Value.Department;
                }
            }
        });
    }
}
}

CentralModel.cs

namespace WpfApplication139.Models
{
public class CentralModel
{
    private WebSocket websocketClient;

    private string url;
    private string protocol;
    private WebSocketVersion version;

    private static CentralModel instance;

    public Dictionary<string, Action<MessageReceivedEventArgs>> Handle { get; set; }
    private CentralModel()
    {
        Handle = new Dictionary<string, Action<MessageReceivedEventArgs>>();
    }
    public void Setup(string url, string protocol, WebSocketVersion version)
    {
        this.url = url;
        this.protocol = protocol;
        this.version = WebSocketVersion.Rfc6455;

        websocketClient = new WebSocket(this.url, this.protocol, this.version);
        websocketClient.MessageReceived += new EventHandler<MessageReceivedEventArgs>(CentralModel.Instance.Message);
    }
    public void Start()
    {
        websocketClient.Open();
    }
    public static CentralModel Instance
    {
        get
        {
            if (instance == null)
            {
                instance = new CentralModel();
            }
            return instance;
        }
    }
    public void Message(object sender, MessageReceivedEventArgs e)
    {
        var dresult = Newtonsoft.Json.JsonConvert.DeserializeObject<Dictionary<string, SomeTypeA>>(e.Message);
        if (Handle.ContainsKey(dresult.Keys.ToList()[0]))
        {
            Handle[dresult.Keys.ToList()[0]](e);
        }
    }
}
}

SomeTypeA and SomeTypeB using for json serealization of two types of messages.

SomeTypeA.cs

public class SomeTypeA
{
    public string Name { get; set; }
    public string Work { get; set; }
    public string ID { get; set; }
    public int value { get; set; }
    public string Department { get; set; }
}

SomeTypeB.cs

public class SomeTypeB
{
    public string Name { get; set; }
    public string Work { get; set; }
    public string ID { get; set; }
    public int value { get; set; }
}
\$\endgroup\$
2
  • \$\begingroup\$ You should consider giving your classes meaningful names. Otherwise it's hard to give you a meaningful feedback. How some1 is supposed to figure out what your code does, when you have classes such as SomeClassB all over your code and no explanations whatsoever. \$\endgroup\$
    – Nikita B
    Commented Sep 1, 2015 at 12:59
  • \$\begingroup\$ First sentenc describe what is going on "I have a wpf application that gets data from webserver." I agree with you that maybe SomeTypeA and SomeTypeB it is not good. But that is not main problem. \$\endgroup\$
    – user75534
    Commented Sep 1, 2015 at 13:08

1 Answer 1

2
\$\begingroup\$

Apart from obvious issues with naming, a few other things caught my eye.

  1. Binding wpf view to objects, which do not implement INotifyPropertyChanged, causes memory leaks. You should always implement it on your view model, even if you don't actually use the event. Same goes for collections and INotifyCollectionChanged.

  2. ObservableCollection should be used on view model level according to MVVM pattern.

  3. Singleton is an anti-pattern in most cases (your case included). You shouldn't use it. Instead you should implement some service (which will act as "model" in MVVM terms) with easy-to-use interface and inject it into your viewmodel. For example:

    interface IEployeeService
    {
        //here you access your server, fetch needed data, etc.
        //your view model should not care about implementation details
        IList<Employee> GetAll();
    }
    
    public class LeftViewModel : INotifyPropertyChanged
    {
        public LeftViewModel(IEployeeService service)
        {
            _service = service;
        }
    
        public ObservableCollection<EmployeeViewModel> Items { get; set; }
    
        public void UpdateCollection()
        {
            var result = _service.GetAll();
            //update Items
            //....
        }
    
        private readonly IEployeeService _service;
    }
    
  4. Dispatcher.Invoke is slow and often causes deadlocks in multithreaded environment. You should use BeginInvoke instead.

  5. I failed to understand the logic behind your "event dictionary", so I can't really comment on that. But it feels like it should not be allowed to exist :)

\$\endgroup\$

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.