1
\$\begingroup\$

I am here to discuss what are the possible improvement can be made in order to make the following code covering all the principles of programming paradigm ( SOLID , DRY ...etc ). Basically, I am practicing Design Patterns.

Example to create multiple types if Cascade Dropdown list of different types ( single select / multiple select items ):

public class MyListType
{
 public string ID { get; set; }
 public string ComplexID { get; set; }
 public string Name { get; set; }
 public bool Selected { get; set; }
}
public class DropDownItem
{
 public string Value { get; set; }
 public string Name { get; set; }
}
public class AdminItem
{
 public string ID { get; set; }
 public string AdminName { get; set; }
}
public class MerchantItem
{
 public string ID { get; set; }
 public string MerchantName { get; set; }
}
public class CustomerItem
{
 public string ID { get; set; }
 public string CustomerName { get; set; }
}
public enum DefaultDDItems
{
 ALL = 0,
 All_Selected = 1,
 NoDefault,
 First_Selected,
 CurrentMonth,
 CurrentYear
}
public interface IDropDown
{
 MultiSelectList GetMultiSelectDropDown();
 MultiSelectList GetMultiSelectDropDown(DefaultDDItems defaultDdItem);
 List<SelectListItem> GetSingleSelectDropDown();
 List<SelectListItem> GetSingleSelectDropDown(DefaultDDItems defaultDdItem);
}
public class AdminLevelDropDown : IDropDown
{
 private List<AdminItem> _adminItemes;
 private string[] _selectedIds;
 public AdminLevelDropDown()
 {
 }
 public AdminLevelDropDown(List<AdminItem> items)
 {
 this._adminItemes = items;
 }
 public AdminLevelDropDown(List<AdminItem> items, string[] defaultSelectedItems)
 {
 this._adminItemes = items;
 this._selectedIds = defaultSelectedItems;
 }
 public MultiSelectList GetMultiSelectDropDown()
 {
 return GetMultiSelectDropDown(DefaultDDItems.NoDefault);
 }
 public MultiSelectList GetMultiSelectDropDown(DefaultDDItems defaultDdItem)
 {
 var mylist = new List<MyListType>();
 if (_adminItemes == null)
 {
 return new MultiSelectList(mylist, "ID", "Name");
 }
 if (!_adminItemes.Any())
 {
 return new MultiSelectList(mylist, "ID", "Name");
 }
 switch (defaultDdItem)
 {
 case DefaultDDItems.ALL:
 mylist.Add(new MyListType()
 {
 Name = "All",
 ID = "0"
 });
 break;
 case DefaultDDItems.All_Selected:
 mylist.Add(new MyListType()
 {
 Name = "All",
 ID = String.Join(",", _adminItemes)
 });
 break;
 // Other switch cases can be implemented here
 }
 mylist.AddRange(_adminItemes.Select(i => new MyListType()
 {
 Name = i.AdminName,
 ID = i.ID
 }).ToList());
 return new MultiSelectList(mylist, "ID", "Name", _selectedIds);
 }
 public List<SelectListItem> GetSingleSelectDropDown()
 {
 return GetSingleSelectDropDown(DefaultDDItems.NoDefault);
 }
 public List<SelectListItem> GetSingleSelectDropDown(DefaultDDItems defaultDdItem)
 {
 var mylist = new List<SelectListItem>();
 if (_adminItemes == null)
 {
 return new List<SelectListItem>(0);
 }
 if (!_adminItemes.Any())
 {
 return new List<SelectListItem>(0);
 }
 switch (defaultDdItem)
 {
 case DefaultDDItems.ALL:
 mylist.Add(new SelectListItem()
 {
 Value = "All",
 Text = "0"
 });
 break;
 case DefaultDDItems.All_Selected:
 mylist.Add(new SelectListItem()
 {
 Value = "All",
 Text = String.Join(",", _adminItemes)
 });
 break;
 }
 mylist.AddRange(_adminItemes.Select(i => new SelectListItem()
 {
 Text = i.AdminName,
 Value = i.ID,
 Selected = _selectedIds.FirstOrDefault() == i.ID
 }).ToList());
 return mylist;
 }
}
public class MerchantLevelDropDown : IDropDown
{
 private List<MerchantItem> _merchantItemes;
 private string[] _selectedIds;
 public MerchantLevelDropDown()
 {
 }
 public MerchantLevelDropDown(List<MerchantItem> items)
 {
 this._merchantItemes = items;
 }
 public MerchantLevelDropDown(List<MerchantItem> items, string[] defaultSelectedItems)
 {
 this._merchantItemes = items;
 this._selectedIds = defaultSelectedItems;
 }
 public MultiSelectList GetMultiSelectDropDown()
 {
 return GetMultiSelectDropDown(DefaultDDItems.NoDefault);
 }
 public MultiSelectList GetMultiSelectDropDown(DefaultDDItems defaultDdItem)
 {
 var mylist = new List<MyListType>();
 if (_merchantItemes == null)
 {
 return new MultiSelectList(mylist, "ID", "Name");
 }
 if (!_merchantItemes.Any())
 {
 return new MultiSelectList(mylist, "ID", "Name");
 }
 switch (defaultDdItem)
 {
 case DefaultDDItems.ALL:
 mylist.Add(new MyListType()
 {
 Name = "All",
 ID = "0"
 });
 break;
 case DefaultDDItems.All_Selected:
 mylist.Add(new MyListType()
 {
 Name = "All",
 ID = String.Join(",", _merchantItemes)
 });
 break;
 }
 mylist.AddRange(_merchantItemes.Select(i => new MyListType()
 {
 Name = i.MerchantName,
 ID = i.ID
 }).ToList());
 return new MultiSelectList(mylist, "ID", "Name", _selectedIds);
 }
 public List<SelectListItem> GetSingleSelectDropDown()
 {
 return GetSingleSelectDropDown(DefaultDDItems.NoDefault);
 }
 public List<SelectListItem> GetSingleSelectDropDown(DefaultDDItems defaultDdItem)
 {
 var mylist = new List<SelectListItem>();
 if (_merchantItemes == null)
 {
 return new List<SelectListItem>(0);
 }
 if (!_merchantItemes.Any())
 {
 return new List<SelectListItem>(0);
 }
 switch (defaultDdItem)
 {
 case DefaultDDItems.ALL:
 mylist.Add(new SelectListItem()
 {
 Value = "All",
 Text = "0"
 });
 break;
 case DefaultDDItems.All_Selected:
 mylist.Add(new SelectListItem()
 {
 Value = "All",
 Text = String.Join(",", _merchantItemes)
 });
 break;
 }
 mylist.AddRange(_merchantItemes.Select(i => new SelectListItem()
 {
 Text = i.MerchantName,
 Value = i.ID,
 Selected = _selectedIds.FirstOrDefault() == i.ID
 }).ToList());
 return mylist;
 }
}
public class CustomerLevelDropDown : IDropDown
{
 private List<CustomerItem> _customerItemes;
 private string[] _selectedIds;
 public CustomerLevelDropDown()
 {
 }
 public CustomerLevelDropDown(List<CustomerItem> items)
 {
 this._customerItemes = items;
 }
 public CustomerLevelDropDown(List<CustomerItem> items, string[] defaultSelectedItems)
 {
 this._customerItemes = items;
 this._selectedIds = defaultSelectedItems;
 }
 public MultiSelectList GetMultiSelectDropDown()
 {
 return GetMultiSelectDropDown(DefaultDDItems.NoDefault);
 }
 public MultiSelectList GetMultiSelectDropDown(DefaultDDItems defaultDdItem)
 {
 var mylist = new List<MyListType>();
 if (_customerItemes == null)
 {
 return new MultiSelectList(mylist, "ID", "Name");
 }
 if (!_customerItemes.Any())
 {
 return new MultiSelectList(mylist, "ID", "Name");
 }
 switch (defaultDdItem)
 {
 case DefaultDDItems.ALL:
 mylist.Add(new MyListType()
 {
 Name = "All",
 ID = "0"
 });
 break;
 case DefaultDDItems.All_Selected:
 mylist.Add(new MyListType()
 {
 Name = "All",
 ID = String.Join(",", _customerItemes)
 });
 break;
 }
 mylist.AddRange(_customerItemes.Select(i => new MyListType()
 {
 Name = i.CustomerName,
 ID = i.ID
 }).ToList());
 return new MultiSelectList(mylist, "ID", "Name", _selectedIds);
 }
 public List<SelectListItem> GetSingleSelectDropDown()
 {
 return GetSingleSelectDropDown(DefaultDDItems.NoDefault);
 }
 public List<SelectListItem> GetSingleSelectDropDown(DefaultDDItems defaultDdItem)
 {
 var mylist = new List<SelectListItem>();
 if (_customerItemes == null)
 {
 return new List<SelectListItem>(0);
 }
 if (!_customerItemes.Any())
 {
 return new List<SelectListItem>(0);
 }
 switch (defaultDdItem)
 {
 case DefaultDDItems.ALL:
 mylist.Add(new SelectListItem()
 {
 Value = "All",
 Text = "0"
 });
 break;
 case DefaultDDItems.All_Selected:
 mylist.Add(new SelectListItem()
 {
 Value = "All",
 Text = String.Join(",", _customerItemes)
 });
 break;
 }
 mylist.AddRange(_customerItemes.Select(i => new SelectListItem()
 {
 Text = i.CustomerName,
 Value = i.ID,
 Selected = _selectedIds.FirstOrDefault() == i.ID
 }).ToList());
 return mylist;
 }
}
public interface IDropdownFactory
{
 AdminLevelDropDown GetMultiSelect_AdminDropDown();
 AdminLevelDropDown GetMultiSelect_AdminDropDown(List<AdminItem> items);
 AdminLevelDropDown GetMultiSelect_AdminDropDown(List<AdminItem> items, string[] selecteditems);
 AdminLevelDropDown GetSingleSelect_AdminDropDown();
 AdminLevelDropDown GetSingleSelect_AdminDropDown(List<AdminItem> items);
 AdminLevelDropDown GetSingleSelect_AdminDropDown(List<AdminItem> items, string[] selectedItems);
 MerchantLevelDropDown GetMultiSelect_MerchantDropDown();
 MerchantLevelDropDown GetMultiSelect_MerchantDropDown(List<MerchantItem> items);
 MerchantLevelDropDown GetMultiSelect_MerchantDropDown(List<MerchantItem> items, string[] selecteditems);
 MerchantLevelDropDown GetSingleSelect_MerchantDropDown();
 MerchantLevelDropDown GetSingleSelect_MerchantDropDown(List<MerchantItem> items);
 MerchantLevelDropDown GetSingleSelect_MerchantDropDown(List<MerchantItem> items, string[] selectedItems);
 CustomerLevelDropDown GetMultiSelect_CustomerDropDown();
 CustomerLevelDropDown GetMultiSelect_CustomerDropDown(List<CustomerItem> items);
 CustomerLevelDropDown GetMultiSelect_CustomerDropDown(List<CustomerItem> items, string[] selecteditems);
 CustomerLevelDropDown GetSingleSelect_CustomerDropDown();
 CustomerLevelDropDown GetSingleSelect_CustomerDropDown(List<CustomerItem> items);
 CustomerLevelDropDown GetSingleSelect_CustomerDropDown(List<CustomerItem> items, string[] selectedItems);
}
public class DropDownFactory : IDropdownFactory
{
 public AdminLevelDropDown GetMultiSelect_AdminDropDown()
 {
 return new AdminLevelDropDown();
 }
 public AdminLevelDropDown GetMultiSelect_AdminDropDown(List<AdminItem> items)
 {
 return new AdminLevelDropDown(items);
 }
 public AdminLevelDropDown GetMultiSelect_AdminDropDown(List<AdminItem> items, string[] selecteditems)
 {
 return new AdminLevelDropDown(items, selecteditems);
 }
 public AdminLevelDropDown GetSingleSelect_AdminDropDown()
 {
 return new AdminLevelDropDown();
 }
 public AdminLevelDropDown GetSingleSelect_AdminDropDown(List<AdminItem> items)
 {
 return new AdminLevelDropDown(items);
 }
 public AdminLevelDropDown GetSingleSelect_AdminDropDown(List<AdminItem> items, string[] selectedItems)
 {
 return new AdminLevelDropDown(items, selectedItems);
 }
 public MerchantLevelDropDown GetMultiSelect_MerchantDropDown()
 {
 return new MerchantLevelDropDown();
 }
 public MerchantLevelDropDown GetMultiSelect_MerchantDropDown(List<MerchantItem> items)
 {
 return new MerchantLevelDropDown(items);
 }
 public MerchantLevelDropDown GetMultiSelect_MerchantDropDown(List<MerchantItem> items, string[] selecteditems)
 {
 return new MerchantLevelDropDown(items, selecteditems);
 }
 public MerchantLevelDropDown GetSingleSelect_MerchantDropDown()
 {
 return new MerchantLevelDropDown();
 }
 public MerchantLevelDropDown GetSingleSelect_MerchantDropDown(List<MerchantItem> items)
 {
 return new MerchantLevelDropDown(items);
 }
 public MerchantLevelDropDown GetSingleSelect_MerchantDropDown(List<MerchantItem> items, string[] selectedItems)
 {
 return new MerchantLevelDropDown(items, selectedItems);
 }
 public CustomerLevelDropDown GetMultiSelect_CustomerDropDown()
 {
 return new CustomerLevelDropDown();
 }
 public CustomerLevelDropDown GetMultiSelect_CustomerDropDown(List<CustomerItem> items)
 {
 return new CustomerLevelDropDown(items);
 }
 public CustomerLevelDropDown GetMultiSelect_CustomerDropDown(List<CustomerItem> items, string[] selecteditems)
 {
 return new CustomerLevelDropDown(items, selecteditems);
 }
 public CustomerLevelDropDown GetSingleSelect_CustomerDropDown()
 {
 return new CustomerLevelDropDown();
 }
 public CustomerLevelDropDown GetSingleSelect_CustomerDropDown(List<CustomerItem> items)
 {
 return new CustomerLevelDropDown(items);
 }
 public CustomerLevelDropDown GetSingleSelect_CustomerDropDown(List<CustomerItem> items, string[] selectedItems)
 {
 return new CustomerLevelDropDown(items, selectedItems);
 }
}

IoC Using AutoFac:

var container = new Container();
container.RegisterType<DropDownFactory>().As<IDropdownFactory>().InstancePerHttpRequest();

Controller:

public class HomeController : Controller
{
 public IDropdownFactory _dropdownFactory;
 public HomeController(IDropdownFactory dropdownFactory)
 {
 this._dropdownFactory = dropdownFactory;
 }
 public ViewResult GetMultiSelect_AdminDropDown(string[] selectedids)
 {
 var adminItems = _adminRepo.GetAdminItems();
 var adminDD = _dropdownFactory.GetMultiSelect_AdminDropDown(adminItems, selectedids);
 return View(adminDD);
 }
 public ViewResult GetSingleSelect_AdminDropDown(string[] selectedids)
 {
 var adminItems = _adminRepo.GetAdminItems();
 var adminDD = _dropdownFactory.GetSingleSelect_AdminDropDown(adminItems, selectedids);
 return View(adminDD);
 }
 public ViewResult GetMultiSelect_MerchantDropDown(string[] selectedids)
 {
 var merchantItems = _merchantRepo.GetMerchantItems();
 var merchantDD = _dropdownFactory.GetMultiSelect_MerchantDropDown(merchantItems, selectedids);
 return View(merchantDD);
 }
 public ViewResult GetSingleSelect_MerchantDropDown(string[] selectedids)
 {
 var merchantItems = _merchantRepo.GetMerchantItems();
 var merchantDD = _dropdownFactory.GetSingleSelect_MerchantDropDown(merchantItems, selectedids);
 return View(merchantDD);
 }
 public ViewResult GetMultiSelect_CustomerDropDown(string[] selectedids)
 {
 var customerItems = _customerRepo.GetCustomerItems();
 var merchantDD = _dropdownFactory.GetMultiSelect_CustomerDropDown(customerItems, selectedids);
 return View(merchantDD);
 }
 public ViewResult GetSingleSelect_CustomerDropDown(string[] selectedids)
 {
 var customerItems = _customerRepo.GetCustomerItems();
 var merchantDD = _dropdownFactory.GetSingleSelect_CustomerDropDown(customerItems, selectedids);
 return View(merchantDD);
 }
}
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Jun 15, 2015 at 12:09
\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

Some quick remarks:

  • Be consistent in your capitalization: DropDownFactory vs IDropdownFactory.

  • GetMultiSelect_AdminDropDown (etc.): DO NOT use underscores, hyphens, or any other nonalphanumeric characters.

  • selecteditems is a compound word, so it should be selectedItems. Ditto selectedids,...

  • DefaultDDItems: Use a singular name for most Enum types, but use a plural name for Enum types that are bit fields. Also: don't use an acronym ("DD"). And why assign values to two of the enum entries, but not all? Also: don't use underscores. And why is ALL all capitals?

  • I see plenty of magic strings that should be constants: "ID", "Name", "All",...

  • There's a typo in _adminItemes. And _merchantItemes.

  • Why does AdminItem have an AdminName, and MerchantItem a MerchantName (etc.)? Why not simply use Name?

  • A lot of this code looks like it's copy-pasted and then a search/replace was executed to change Admin to Merchant etc. -- hence the same typos appearing in multiple places. Once you do that, you should realize that you need to make this piece of code more abstract and for instance put it in a base class.

  • DropDownFactory isn't a factory how I understand the concept.

answered Jun 17, 2015 at 9:37
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.