开发者

how do i refactor this code?

i have .net 3.5 and i would like to make a generic method. how do i refactor this code?

            case (int)Enums.SandwichesHoagies.Cheeses:
                if (this.Cheeses.Where(x => x.Id == product.ProductId).SingleOrDefault() == null)
                {
                    var newCheese = new Cheese
                    {
                        Id = product.ProductId,
                        Name = product.Name,
                        PriceValue = product.Price.HasValue ? (double)product.Price.Value : 0.00
                    };

                    this.Cheeses.Add(newCheese);
                }
                else
                {
                    foreach (var cheese in this.Cheeses.Where(cheese => cheese.Id == product.ProductId))
                    {
                        this.Cheeses.Remove(cheese);
                        break;
                    }
                }

                foreach (var cheese in Cheeses) cheese.Type = string.Empty;

                if (this.Cheeses.Count > 0) Cheeses.First().Type = "Cheeses:";

                break;

            case (int)Enums.SandwichesHoagies.Meats:
                if (this.Meats.Where(x => x.Id == product.ProductId).SingleOrDefault() == null)
                {
                    var newMeat = new Meat
                    {
                        Id = product.ProductId,
                        Name = product.Name,
                        PriceValue = product.Price.HasValue ? (double)product.Price.Value : 0.00
                    };

                    this.Meats.Add(newMeat);
                }
                else
                {
                    foreach (var meat in this.Meats.Where(meat => meat.Id == product.ProductId))
                    {
                        this.Meats.Remove(meat);
                        break开发者_如何转开发;
                    }
                }

                foreach (var meat in Meats) meat.Type = string.Empty;

                if (this.Meats.Count > 0) Meats.First().Type = "Meats:";

                break;


Assuming two things:

  1. Meat and Cheese inherit from Ingredient or implement IIngredient
  2. The Meats and Cheeses collections are IList<T>

Here we go:

private void OuterMethod()
{
   switch(something)
   {
       case (int)Enums.SandwichesHoagies.Cheeses:
           HandleCase(product, this.Cheeses);
           break;
       case (int)Enums.SandwichesHoagies.Meats:
           HandleCase(product, this.Meats);
           break;
   }
}

private void HandleCase<T>(Product product, List<T> list) where T : Ingredient, new()
{
    if(list.Any(i => i.Id == product.ProductId))
    {
        list.Add(new T {
            Id = product.ProductId,
            Name = product.Name,
            PriceValue = product.PriceValue ?? 0.0;
        });
    }
    else
    {
        list.RemoveAll(i => i.Id == product.ProductId);
    }

    //NOTE: this part seems like a bad idea. looks like code smell.
    foreach (var i in list)
    {
        i.Type = string.Empty;
    }
    if (list.Count > 0)
    {
        list.First().Type = "Cheeses:";
    }
}


At initial glance, you have some common properties you access, Id, Name, PriceValue, and Type. That looks like a base class or interface to me. With that, you could start by refactoring your code into a method

void YourMethod<T>(List<T> list, Product product) where T : IProduct, new() 
// IProduct being your interface or base class

In which case, where you refer to this.Meats or this.Cheeses, you would instead refer to list, and where you refer to instances of Meat or Cheese, you simply refer to T.

See how far that gets you and refactor further.


Hard to know your exact requirements without knowing the types (and base types/interfaces) used. I'm going to assume you're using some kind of ORM which spits out partial classes anyway.

First requirement to make this easy to work with, is that Meat and Cheese share a common interface (or abstract class). This should be something basic like this.

interface IProduct {
    int Id { get; set; }
    String Name { get; set; }
    Double PriceValue { get; set; }
}

The assumption that partial classes are used makes it easy to extend your class to use this interface.

partial class Cheese : IProduct { }

I find it interesting that you have a different kind of Product which has different field names, but almost the same features. Should you perhaps keep the names the same as the above interface and make this also derived from the interface? Anyway, assuming what you have is something like this.

class Product {
    int ProductId { get; set; }
    String Name { get; set; }
    Price? Price { get; set; }
}

The first thing you wanna do is use the Factory Pattern to create a specific product.

public class ProductFactory {
    public T Create<T>(Product product) 
      where T : IProduct, new() {
        return new T {
            Id = product.ProductId,
            Name = product.Name,
            PriceValue = product.Price.HasValue 
                ? (double)product.Price.Value 
                : 0.00    
        };
    }
}

The new() constraint requires a parameterless constructor in your Cheese and Meat classes. I assume this is no problem. You just need to call .Create<Cheese>(product);

The next parts, I would need to assume that your Cheeses and Meats objects (or properties), also share a common class (ICollection<IProduct>), or you could define your own for particular needs.

public class ProductCollection : ICollection<IProduct> { ... }

A generic method to check if a product exists

Boolean ContainsProduct<T>(ProductCollection<T> collection, Product product) 
  where T : IProduct {
    return collection.Where(x => x.Id == product.Id).SingleOrDefault != null;
}

I'm skeptical of your idea of calling .Remove inside a foreach loop. Modifying a collection can cause problems for the enumerator being used to loop through it. I would find a better approach to that if it's a concern.

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜