Click here to Skip to main content
15,882,055 members
Please Sign up or sign in to vote.
1.00/5 (3 votes)
See more:
Hi Can some one suggest me the Refactoring for the below code as I am looking for how to approach to fix this bad code as an exercise.  

public class OrderManager
    {
        private readonly IOrderStore orderStore;

        public OrderManager(IOrderStore orderStore)
        {
            this.orderStore = orderStore;
        }

        public void WriteOutSmallOrders()
        {
            var orders = orderStore.GetOrders();
            SmallOrderFilter filter = new SmallOrderFilter(new OrderWriter(), orders);
            filter.WriteOutFiltrdAndPriceSortedOrders(new OrderWriter());
        }

        public void WriteOutLargeOrders()
        {
            var orders = orderStore.GetOrders();
            LargeOrderFilter filter = new LargeOrderFilter(new OrderWriter(), orders);
            filter.WriteOutFiltrdAndPriceSortedOrders(new OrderWriter());
        }
    }


    public class LargeOrderFilter
    {
        private IOrderWriter orderWriter;
        private List<Order> orders;

        public LargeOrderFilter(IOrderWriter orderWriter, List<Order> orders)
        {
            filterSize = "100";
            this.orderWriter = orderWriter;
            this.orders = orders;
        }

        protected string filterSize;

        public void WriteOutFiltrdAndPriceSortedOrders(IOrderWriter writer)
        {
            List<Order> filteredOrders = this.FilterOrdersSmallerThan(orders, filterSize);
            Enumerable.OrderBy(filteredOrders, o => o.Price);

            ObservableCollection<Order> observableCollection =
                new ObservableCollection<Order>();

            foreach (Order o in filteredOrders)
            {
                observableCollection.Add(o);
            }

            writer.WriteOrders(observableCollection);
        }

        protected List<Order> FilterOrdersSmallerThan(List<Order> allOrders, string size)
        {
            List<Order> filtered = new List<Order>();
            for (int i = 0; i <= allOrders.Count; i++)
            {
                int number = orders[i].toNumber(size);

                if (allOrders[i].Size <= number)
                {
                    continue;
                }
                else
                {
                    filtered.Add(orders[i]);
                }
            }

            return filtered;
        }
    }

    public class SmallOrderFilter : LargeOrderFilter
    {
        public SmallOrderFilter(IOrderWriter orderWriter, List<Order> orders)
            : base(orderWriter, orders)
        {
            filterSize = "10";
        }
    }


    public class Order
    {
        public double Price
        {
            get { return this.dPrice; }
            set { this.dPrice = value; }
        }

        public int Size
        {
            get { return this.iSize; }
            set { this.iSize = value; }
        }

        public string Symbol
        {
            get { return this.sSymbol; }
            set { this.sSymbol = value; }
        }

        private double dPrice;
        private int iSize;
        private string sSymbol;

        public int toNumber(String Input)
        {
            bool canBeConverted = false;
            int n = 0;
            try
            {
                n = Convert.ToInt32(Input);
                if (n != 0) canBeConverted = true;
            }
            catch (Exception ex)
            {
            }

            if (canBeConverted == true)
            {
                return n;
            }
            else
            {
                return 0;
            }
        }
    }


    // These are stub interfaces that already exist in the system
    // They're out of scope of the code review
    public interface IOrderWriter
    {
        void WriteOrders(IEnumerable<Order> orders);
    }

    public class OrderWriter : IOrderWriter
    {
        public void WriteOrders(IEnumerable<Order> orders)
        {
        }
    }

    public interface IOrderStore
    {
        List<Order> GetOrders();
    }


    public class OrderStore : IOrderStore
    {
        public List<Order> GetOrders()
        {
            return new List<Order> { new Order {
                Price = 10,
                Size =1,
                Symbol = "TShirt"
            }, new Order {
                Price = 15,
                Size =2,
                Symbol = "Sport Goods"
            } };
        }
    }


What I have tried:

SmallOrderFilter filter = new SmallOrderFilter(new OrderWriter(), orders);

move the above to a constructor and inject
Remove repetitive code
create an abstract class
Posted
Updated 22-Jan-18 22:36pm
Comments
Paulo Zemek 22-Jan-18 16:28pm    
I think if this is an exercise for you, you should write the code.

But I can give you some hints:
See that there is WriteOutSmallOrders and WriteOutLargeOrders? What about a WriteOutOrders(filter) ?

But then, how do you make the filter work? You will need either an abstract class or an interface (IFilter).

In fact, there's already an issue. SmallOrderFilter shouldn't inherit from LargerOrderFilter... or else, you can pass a SmallOrderFilter in any place that explicitly asks for a LargeOrderFilter... quite odd, don't you think?

Also, even the LargeOrderFilter is limited to 100... and, why is it "100" (that is, a string) instead than 100 (an integer) ?

1 solution

We don't do your homework or exercises for you - exercises are set to help you learn and to cement that new knowledge into your brain.

So if we just gave you all the suggestions then we would be doing you a disfavour.

You already have some suggestions that you haven't applied yet:
Quote:
move the above to a constructor and inject
Remove repetitive code
create an abstract class

@Paulo-Zemek has given you some other things to look at.

Here are a couple of more lines to look at carefully ... these are just silly mistakes, see if you can work out why:
C#
if (canBeConverted == true)

C#
catch (Exception ex)
{
}
 
Share this answer
 

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900