3
\$\begingroup\$

I was given task to build a client server application, using any technology I want. The task was to build a database(doesn't have to be a real database, it can be mocked). the client side should support more than one user/

I didn't use a real database I just created some shares and created a mechanism to update them from time to time using a random value.

I initialized the database with 2 users, I don't need to add users or delete them, just show I can support more than one.

Since I have a background in C# and WPF, I created 3 projects:
1. WPF/MVVM client side 2. common library 3. WebAPI - server side, which "includes" the database.

I would like you to please comment about the correctness of my implementation as if it was a code review for your team. OOP design, usage of client server, please don't take into account I did it with WPF. Assume you have half a day to work on the project and then submit.

I would appreciate any comments or questions.

1. WPF project/ MVVM - I used mvvm light tool kit

MainWindow.xaml

<Window x:Class="Client.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        Title="MainWindow" Height="350" Width="525">
    <Grid>
        <Grid.RowDefinitions>
            <RowDefinition Height="auto" />
            <RowDefinition Height="auto" />
            <RowDefinition Height="auto" />
        </Grid.RowDefinitions>
        <Grid.ColumnDefinitions>
            <ColumnDefinition Width="Auto"></ColumnDefinition>
            <ColumnDefinition Width="Auto"></ColumnDefinition>
        </Grid.ColumnDefinitions>
        <TextBlock Grid.Row="0" Grid.Column="0" Text="enter use name: " MinWidth="75"/>
        <TextBox Grid.Row="0" Grid.Column="1" MinWidth="75" Text="{Binding UserName,Mode=TwoWay,UpdateSourceTrigger=PropertyChanged}"/>
        <Button Grid.Row="1" Grid.Column="0" Content="Get All Shares" Command="{Binding GetAllSharesCommand,Mode=TwoWay}"/>
        <Button Grid.Row="1" Grid.Column="1" Content="Get My Shares" Command="{Binding GetSharePerUserCommand,Mode=TwoWay}"/>
        <DataGrid Grid.Row="2" ItemsSource="{Binding Shares}">

        </DataGrid>
    </Grid>
</Window>

HttpHandler.cs

namespace Client
{
    public class HttpHandler
    {
        private HttpClient client;
        public HttpHandler()
        {
            client = new HttpClient();
            client.BaseAddress = new Uri("http://localhost:18702/");
            client.DefaultRequestHeaders.Accept.Clear();
            client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
        }

        public async Task<IEnumerable<Share>> GetallSharesAsync(string path)
        {
            IEnumerable<Share> shares = null;
            HttpResponseMessage response = await client.GetAsync(path);
            if (response.IsSuccessStatusCode)
            {
                shares = await response.Content.ReadAsAsync<IEnumerable<Share>>();
            }
            return shares;
        }

        public async Task<IEnumerable<Share>> GetSharePerUserAsync(string path)
        {
            IEnumerable<Share> shares = null;
            HttpResponseMessage response = await client.GetAsync(path);
            if (response.IsSuccessStatusCode)
            {
                shares = await response.Content.ReadAsAsync<IEnumerable<Share>>();
            }
            return shares;
        }

        public async Task<IDictionary<string, int>> GetAllUsersAsync(string path)
        {
            IDictionary<string, int> users2Id = null;
            HttpResponseMessage response = await client.GetAsync(path);
            if (response.IsSuccessStatusCode)
            {
                users2Id = await response.Content.ReadAsAsync<IDictionary<string, int>>();
            }
            return users2Id;
        }
    }
}

ClientViewModel.cs

namespace Client
{
    public class ClientViewModel : ViewModelBase
    {
        private ObservableCollection<Share> _shares;
        public ObservableCollection<Share> Shares
        {
            get { return _shares; }
            set { _shares = value; }
        }

        private string _userName;
        public string UserName
        {
            get { return _userName; }
            set
            {
                _userName = value;
                RaisePropertyChanged("UserName");
                GetAllSharesCommand.RaiseCanExecuteChanged();
            }
        }
        private RelayCommand _getAllSharesCommand;
        public RelayCommand GetAllSharesCommand
        {
            get { return _getAllSharesCommand; }
            set
            {
                _getAllSharesCommand = value;
            }
        }
        private RelayCommand _GetSharesPerUserCommand;

        public RelayCommand GetSharePerUserCommand
        {
            get { return _GetSharesPerUserCommand; }
            set { _GetSharesPerUserCommand = value; }
        }

        private HttpHandler handler;
        private Dictionary<string, int> _userName2Id;
        public ClientViewModel()
        {
            GetAllSharesCommand = new RelayCommand(ExecuteGetAllShares, CanExecuteGetAllShares);
            GetSharePerUserCommand = new RelayCommand(ExecuteGetSharePerUserCommand, CanExecuteGetSharePerUserCommand);

            handler = new HttpHandler();
            Shares = new ObservableCollection<Share>();
            GetUsers();
        }

        private async void GetUsers()
        {
            IDictionary<string, int> userNames2ID = await handler.GetAllUsersAsync("api/users");
            _userName2Id = new Dictionary<string, int>(userNames2ID);
        }

        private bool CanExecuteGetSharePerUserCommand()
        {
            return !String.IsNullOrEmpty(UserName);
        }

        private async void ExecuteGetSharePerUserCommand()
        {
            string temp = "api/shares" + "/" + _userName2Id[UserName];
            try
            {
                IEnumerable<Share> tempShares = await handler.GetSharePerUserAsync(temp);
                Shares.Clear();
                foreach (var item in tempShares)
                {
                    Shares.Add(item);
                }
            }
            catch (Exception)
            {
                throw;
            }
        }
        public bool CanExecuteGetAllShares()
        {
            return !String.IsNullOrEmpty(UserName);
        }
        public async void ExecuteGetAllShares()
        {
            try
            {
                IEnumerable<Share> tempShares = await handler.GetallSharesAsync("api/shares");
                Shares.Clear();
                foreach (var item in tempShares)
                {
                    Shares.Add(item);
                }
            }
            catch (Exception)
            {
                throw;
            }
        }
    }
}

2.Common - project

Share.cs

namespace Common
{
    public class Share
    {
        public int Id { get; set; }
        public string Name { get; set; }
        public double Price { get; set; }
    }
}

3.Server - WebApi project(yea I know what a great name)

WebApiConfig.cs

namespace SharesApp
{
    public static class WebApiConfig
    {
        public static void Register(HttpConfiguration config)
        {
            config.MapHttpAttributeRoutes();

            config.Routes.MapHttpRoute(
                name: "DefaultApi",
                routeTemplate: "api/{controller}/{id}",
                defaults: new { id = RouteParameter.Optional }
            );
        }
    }
}

Controllers folder

SharesController.cs

namespace SharesApp.Controllers
{
    public class SharesController : ApiController
    {
        //this is a mock for a real database, i'm not sure where do I need to connect to the real DB        
        private static IDataBase _dataBase;
        public SharesController()
        {
            if (_dataBase == null)
            {
                _dataBase = new SharesDataBase();
            }
        }
        public IEnumerable<Share> GetAllShares()
        {
            try
            {
                return _dataBase.GetAllShares();
            }
            catch (Exception)
            {

                throw;
            }
        }
        public IHttpActionResult GetUpdatedShares(int id)
        {
            IEnumerable<Share> share = null;
            try
            {
                share = _dataBase.GetShareById(id);
            }
            catch (Exception)
            {

                throw;
            }
            if (share == null)
            {
                return NotFound();
            }
            return Ok(share);
        }   
    }

UsersController .cs

namespace SharesApp.Controllers
{
    public class UsersController : ApiController
    {
        private Dictionary<string, int> _userName2Id;
        public UsersController()
        {
            _userName2Id = new Dictionary<string, int>();
            _userName2Id.Add("user10", 1);
            _userName2Id.Add("user20", 2);
        }
        public IDictionary<string, int> GetAllUserNames()
        {
            return _userName2Id;
        }
        public string GetUserNameById(int id)
        {
            if (!_userName2Id.ContainsValue(id))
            {
                return null;
            }
            return _userName2Id.FirstOrDefault(x => x.Value == id).Key;
        }
    }
}

Models folder

IDataBase.cs

namespace SharesApp.Models
{
    public interface IDataBase
    {
        IEnumerable<Share> GetAllShares();
        IEnumerable<Share> GetShareById(int id);

    }
}

SharesDataBase.cs

namespace SharesApp.Models
{
    public class SharesDataBase : IDataBase
    {
        //user name to list of shares names

        const string INTC = "INTC";
        const string MSFT = "MSFT";
        const string TEVA = "TEVA";
        const string YAHOO = "YAHOO";
        const string P500 = "P500";
        private List<Share> _shares;
        private Random _random;
        private int _maximum = 100;
        private int _minimum = 1;
        public Dictionary<int, List<string>> User2Shares { get; set; }

        private Object thisLock = new Object();  

        public SharesDataBase()
        {
            _random = new Random();
            User2Shares = new Dictionary<int, List<string>>();
            //init the shares list
            _shares = new List<Share>
        { 
            new Share { Id = 1, Name = INTC, Price = 1 }, 
            new Share { Id = 2, Name = MSFT, Price = 3.75 }, 
            new Share { Id = 3, Name = TEVA,  Price = 16.99},
            new Share { Id = 4, Name = YAHOO,  Price = 11.0},
            new Share { Id = 5, Name = P500,  Price = 5.55},
        };


            //init the users
            User2Shares.Add(1, new List<string>() { INTC, MSFT, TEVA });
            User2Shares.Add(2, new List<string>() { YAHOO, P500, TEVA });
            Task.Run(()=>UpdateShares());


        }
        private void UpdateShares()
        {
            while (true)
            {
                System.Threading.Thread.Sleep(1000);// wait for 1 sec
                lock (thisLock)
                {
                    foreach (var item in _shares)
                    {
                        int tempRandom = _random.Next(1, 1000);
                        if (tempRandom % 100 == 0)
                        {
                            item.Price = _random.NextDouble() * (_maximum - _minimum) + _minimum;
                        }
                    }
                }
            }
        }
        public IEnumerable<Share> GetAllShares()
        {
            return _shares;
        }

        public IEnumerable<Share> GetShareById(int id)
        {
            if (!User2Shares.ContainsKey(id))
            {
                return null;
            }
            var listOfShares = User2Shares[id];
            if (listOfShares.Count == 0)
            {
                //this userName doesn't have any shares
                return null;
            }
            List<Share> sharesList = new List<Share>();
            foreach (var name in listOfShares)
            {
                var res = _shares.FirstOrDefault(x => x.Name == name);
                if (res != null)
                {
                    sharesList.Add(res);
                }

                //share is missing from the server
            }
            return sharesList;
        }
    }
\$\endgroup\$
  • \$\begingroup\$ This isn't an interview question, is it? \$\endgroup\$ – t3chb0t Jul 9 '17 at 17:29
  • \$\begingroup\$ No. It is a task that a friend who works in the web domain which I have no knowledge of. Gave me as a practice to ramp up on web api and client server \$\endgroup\$ – Gilad Jul 9 '17 at 17:50
2
\$\begingroup\$

ClientViewModel

If you don't validate property setters, meaning you just store the setted value in a backing variable, you should use autoimplemented properties like so

public RelayCommand GetAllSharesCommand
{
    get; set;
}  

which makes the code shorter and more readable.


You should only catch exceptions which you can/want to handle. If you have

catch (Exception)
{
    throw;
}  

you can just ommit the try..catch because you will handle the exception at another place. Here the try..catch only adds noise to your code.

As an example, the ExecuteGetAllShares() method would look like so

public async void ExecuteGetAllShares()
{
    IEnumerable<Share> tempShares = await handler.GetallSharesAsync("api/shares");
    Shares.Clear();
    foreach (var item in tempShares)
    {
        Shares.Add(item);
    }
}  

and would do the exact same thing.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ I thought when accessing an external content you should always use try catch. but ok. \$\endgroup\$ – Gilad Jul 10 '17 at 12:39
  • \$\begingroup\$ Using try..catch is good as long as you do something with the exception. Either logging it or handling it. Just throwing it is the same as omitting try..catch. \$\endgroup\$ – Heslacher Jul 10 '17 at 12:42
  • \$\begingroup\$ ok sure, this is what I meant to add logging, I had 4 hours so I didn't use Log4net or something like that, but I get your idea \$\endgroup\$ – Gilad Jul 10 '17 at 12:43

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.