Дилемма статического класса, касающаяся только статических методов - PullRequest
0 голосов
/ 10 октября 2011

Итак, у меня есть задача составить карту двух каталогов отелей;оба являются CSV-файлами.Я создал два класса, основываясь на их обязанностях: 1. CatalogManager: обрабатывает операции ввода-вывода для каталогов.2. CatalogMapper: обрабатывает задачу отображения двух каталогов.

Определения следующие:

public static class CatalogManager
{
   public static List<Hotel> GetHotels(string filePath) { }
   public static void SaveHotels (List<Hotel> hotels, string filePath) { }
   public static void SaveMappedHotels (List<MappedHotel> hotels, string filePath) { }
   public static List<string> GetHotelChains(string filePath) { }
}

public static class CatalogMapper
{
   public static List<MappedHotel> MapCatalogs (List<Hotel> masterCatalog, List<Hotel> targetCatalog) { }

   public static FetchAddressGeoCodes (Hotel.Address address)
   { // fetch address's geocode using Google Maps API }

   public static string GetRelevantHotelChain (string hotelName)
   {
      List<string> chains = CatalogManager.GetChains();
      // find and return the chain corresponding to hotelName. 
   }
}

Типичная операция отображения может выглядеть примерно так:

List<Hotel> masterCatalog = CatalogManager.GetHotels(masterFilePath);
List<Hotel> targetCatalog = CatalogManager.GetHotels(targetFilePath);
List<MappedHotel> mappedHotels = CatalogMapper.MapHotels(masterCatalog, targetCatalog);
CatalogManager.SaveMappedHotels(mappedHotels, mappedCatalogFilePath);

Как показывает код, оба класса являются статическими.Хотя я нашел их правильными и работающими, я все еще чувствую, что с этим дизайном что-то не так в смысле ООП.Хорошо, что оба класса просто статичны?Я не нашел необходимости в их создании.Кроме того, каковы другие недостатки в этом дизайне?Я уверен, что недостатки присутствуют.Каковы решения для тех?

Ответы [ 2 ]

1 голос
/ 10 октября 2011

Не бойся свободной функции!

Мне кажется подозрительным, что CatalogMapper отображает Hotel.Address.Чтобы быть должным образом учтенным / инкапсулированным, либо

  • CatalogMapper должно работать на не-специфичных для отеля Address,

  • или,если Hotel.Address является каким-то особым для GeoCodes, то Hotel.Address должен быть в состоянии сопоставить себя с GeoCode без CatalogMapper.


С пояснениями в комментариях яЯ хотел бы предложить следующее.Я не знаю C #, поэтому я напишу это как C ++.Я надеюсь, что это переводит

struct Hotel {
    const Address & address () const;

    // I typedef EVERTYTHING :-)    
    typedef std :: list <std :: string> StringList;
    typedef std :: pair <Hotel, Hotel> Pair;
    typedef std :: list <Hotel> Container; // TODO implicit sharing?
    typedef std :: list <Pair> Mapping;

    // NB will be implemented in terms of std::istream operations below,
    // or whatever the C# equivalent is.
    // These could arguably live elsewhere.
    static Container load (const std :: string &);
    static Mapping load_mapping (const std :: string &);
    static void save (const std :: string &, const Container &);
    static void save_mapping (const std :: string &, const Mapping &);

    // No need for a "Manager" class for this.
    static StringList load_chain (const string & file_name);

private:
    static Hotel load (std :: istream &);
    void save (std :: ostream &) const;
};

// Global namespace, OK because of overloading. If there is some corresponding
// generic library function which merges pair-of-list into list-of-pair
// then we should be specialising/overloading that.
Hotel :: Mapping merge (const Hotel :: Container &, const Hotel :: Container &);

struct GeoCode {
   GeoCode (const Address &);
};

Всякий раз, когда я вижу класс, называемый «Менеджер», если это не объект, который создает, владеет и контролирует доступ к другим объектам, то это предупреждение о том, что мы вКоролевство Существительных.Объекты могут управлять собой.У вас должен быть отдельный класс для логики, только если эта логика выходит за рамки одного класса.

1 голос
/ 10 октября 2011

Другим разумным подходом было бы сделать CatalogManager нестатический класс, инициализированный именем файла.Это позволит вам использовать файлы частично из памяти и читать или записывать, когда это необходимо.

...