Получение элементов списка


Есть ли возможность рефакторинга этого кода?

В классе а:

public List<Item> GetItems() {
  var result = new List<Item>();
  foreach(var item in repo.GetItems1()) {
    var x = repo.GetOtherItems1(item.Id, "param1", param2); // this part is different
    if (x.Value > 5)
      result.Add(x);
  }  

  return result;
}

В классе "Б":

public List<Item> GetItems() {
  var result = new List<Item>();
  foreach(var item in repo.GetItems2()) {
    var x = repo.GetOtherItems2(param1, param2, item.Id); // this part is different
    if (x.Value > 5)
      result.Add(x);
  }  

  return result;
}

Я пытался использовать шаблонный метод, но из-за других параметров в GetOtherItemsX(...), это сейчас возможно?



587
7
c#
задан 12 декабря 2011 в 10:12 Источник Поделиться
Комментарии
4 ответа

Ну, вы могли бы получить делегат для получения элементов, то есть сделать действие, которое отличается:

public List<Item> GetItems(Func<Item, Repository> getOtherItems) {
if( getOtherItems == null ) {
throw new ArgumentException();
}
var result = new List<Item>();
foreach(var item in repo.GetItems2()) {
var x = getOtherItems();
if (x.Value > 5)
result.Add(x);
}

return result;
}

И тогда вы могли бы вызвать его так:

// In class A
this.GetItems( repo => repo.GetOtherItems1(item.Id, "param1", param2) );

// In class B
this.GetItems( repo => repo.GetOtherItems2(param1, param2, item.Id) );


Но я заметил, что вы также использовать РЕПО.GetItems1() В и РЕПО.GetItems2() в Б.
Так что разницу вы указали не единственный.

С учетом этого, я бы посоветовал переосмыслить структуру класса. Эти двое могли наследовать от базового класса, и просто продлить его. (Или это может быть полиморфизм, или...)

Может быть:

public interface IMyList<Item>
{
List<int> GetItems();
// If it returns a single item, the name should NOT be pluralized!!
Item GetOtherItems();
}

Или может быть абстрактным классом?


редактировать: читать @JoeGeeky комментарий влияние на производительность с помощью делегатов, так как это может стать актуальным, если делегат используется на очень интенсивные цикла или при высоких нагрузках.

11
ответ дан 12 декабря 2011 в 11:12 Источник Поделиться

Я хотел бы сделать несколько предложений:


  • Рассмотреть возможность возвращения типа GetItems методов класса IEnumerable, Интерфейс ICollectionили объекта IListв зависимости от того , как вы собираетесь использовать результат функции. Это позволит впоследствии легче рефакторинга.

  • Рассмотреть вопрос о переименовании вашего РЕПО.GetOtherItemsN методов. Их имена предполагают, что несколько элементов были возвращены, а не один пункт, но они, кажется, возвращаясь отдельных элементов. (ANeves упоминает об этом в комментариях для его интерфейса)

  • Рассмотреть вопрос об обновлении подписи обоих РЕПО.GetOtherItemsN методы принимают параметры в аналогичном порядке. Кажется странным, что человек принимает в ИД, тогда параметр1 (как строку), затем параметр param2, в то время как другой берет на параметр1 (неизвестного типа), то параметр param2, затем ИД. Код, вероятно, должен прийти первым.

  • Рекомендуется использовать постоянное значение enum в место 5 в вашем случае, если (x.Значение > 5) заявления. Если оба класса A и B имеют общий базовый класс (предложил, учитывая другие ответы), я бы поставил его там.

  • Опираясь на ANeves ответ, я бы также передать в интерфейс IEnumerable параметр GetItems функция, поскольку вы используете РЕПО.GetItems1 против РЕПО.GetItems2 в два класса, и вы не делаете ничего больше, чем перечисление коллекции.

Если вы находитесь в LINQ-вентилятор, вы также можете превратить это в инструкции LINQ:

public IEnumerable<Item> GetItems(IEnumerable<Items> source, Func<Item, Repository> getOtherItems) 
{
if (getOtherItems == null)
{
throw new ArgumentException();
}
var result = from item in source
let x = getOtherItems()
where x.Value > MIN_VAL
select x;
}

Если вам это нужно оценить, а не ленивый загружен, нужно просто бросить на .Вызова метода toList () или .Метод toArray() перед возвращением.

5
ответ дан 12 декабря 2011 в 06:12 Источник Поделиться

Создать класс, который держит свои параметры, и вместо передачи отдельных параметров, переданных в объект класса. что-то вроде:

class MyClass
{

public MyClass(){}

public int ItemId { get; set;}
public string ItemParam1 {get; set;}
....
}

0
ответ дан 12 декабря 2011 в 11:12 Источник Поделиться

После приема Дэн и другие предложения по очистке интерфейс репозитория, вы могли бы также иметь некоторые преимущества от использования общего выражения для преобразования языка LINQ, где заявление.

[TestMethod]
public void CanUseCommonWhereExpression()
{
var repo = SetupRepo();

Assert.AreEqual(1, GetItems1(repo).Where(ValueMoreThan(5)).Count());
Assert.AreEqual(2, GetItems2(repo).Where(ValueMoreThan(5)).Count());
Assert.AreEqual(
2,
GetItems2(repo).AsQueryable().Where(QueryableValueMoreThan(5)).Count());
}

// Fine for in-memory linq
private Func<Item, bool> ValueMoreThan(int value)
{
return i => i.Value > value;
}

// Also fine for database query translation
private Expression<Func<Item, bool>> QueryableValueMoreThan(int value)
{
return i => i.Value > value;
}

private static IEnumerable<Item> GetItems2(IRepository repo)
{
return repo.GetItems2().SelectMany(i => repo.GetOtherItems2(i.Id, 0));
}

private static IEnumerable<Item> GetItems1(IRepository repo)
{
return repo.GetItems1().SelectMany(i => repo.GetOtherItems1(0, i.Id));
}

private static IRepository SetupRepo()
{
var repo = MockRepository.GenerateMock<IRepository>();
repo.Stub(r => r.GetItems1()).Return(new[] { new Item { Id = 1, Value = 3 } });
repo.Stub(
r => r.GetOtherItems1(0, 1)).Return(new[] { new Item { Id = 2, Value = 3 },
new Item { Id = 3, Value = 6 } });
repo.Stub(r => r.GetItems2()).Return(new[] { new Item { Id = 4, Value = 3 } });
repo.Stub(
r => r.GetOtherItems2(4, 0)).Return(new[] { new Item { Id = 5, Value = 6 },
new Item { Id = 6, Value = 7 } });
return repo;
}

public interface IRepository
{
IEnumerable<Item> GetItems1();
IEnumerable<Item> GetOtherItems1(int foo, int id);
IEnumerable<Item> GetItems2();
IEnumerable<Item> GetOtherItems2(int id, int bar);
}

public class Item
{
public int Id { get; set; }
public int Value { get; set; }
}

0
ответ дан 13 декабря 2011 в 01:12 Источник Поделиться