静态类仅包含静态方法的困境

发布于 2024-12-09 04:14:40 字数 1483 浏览 0 评论 0原文

所以我的任务是映射两个酒店目录;两者都是 csv 文件。 我根据它们的职责创建了两个类: 1. CatalogManager :处理目录的 I/O 操作。 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);

如代码所示,这两个类都是静态的。虽然我发现它们是正确的并且可以工作,但我仍然觉得这个设计在 OOP 方面有问题。 这两个类都是静态的可以吗?我发现不需要实例化它们。 另外,这个设计还有哪些缺陷?我确信存在缺陷。针对这些问题有哪些解决方案呢?

So I have this task of mapping two hotel catalogs; both are csv files.
I created two classes, based on their responsibilities :
1. CatalogManager : handles the I/O operations for catalogs.
2. CatalogMapper : Handles the mapping task of two catalogs.

The definitions are as follows :

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. 
   }
}

A typical mapping operation may go something like :

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

As the code shows, both the classes are static. Though I found them right and working, I still feel there is something wrong with this design in terms of OOP.
Is it fine that both of the classes are simply static? I found no need of instantiating them.
Also, what are other flaws in this design? I am sure flaws are present. What are the solutions for those?

如果你对这篇内容有疑问,欢迎到本站社区发帖提问 参与讨论,获取更多帮助,或者扫码二维码加入 Web 技术交流群。

扫码二维码加入Web技术交流群

发布评论

需要 登录 才能够评论, 你可以免费 注册 一个本站的账号。

评论(2

澜川若宁 2024-12-16 04:14:40

另一种合理的方法是使 CatalogManager 成为一个使用文件名初始化的非静态类。这将允许您部分使用内存中的文件,并在需要时读取或写入。

Another reasonable approach would be to make CatalogManager a non-static class initialized with the file-name. This would allow you to use the files partially from memory and read or write when its required.

断舍离 2024-12-16 04:14:40

不要害怕免费功能!

我发现CatalogMapper很可疑映射Hotel.Address。要正确分解/封装,

  • CatalogMapper 应该在非酒店特定的 Address 上运行,

  • 或者,如果 Hotel.Address 在地理编码方面有某种特殊性,则 Hotel.Address 应该能够将自身映射到没有 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 &);
};

每当我看到一个名为“Manager”的类时,如果它不是一个创建、拥有和控制对其他对象的访问的对象,那么它就是一个警告,我们正处于名词王国中。对象可以自我管理。如果该逻辑超出了单个类的范围,那么您应该只为该逻辑创建一个单独的类。

Fear not the free function!

I find it suspicious that CatalogMapper maps a Hotel.Address. To be properly factored/encapsulated, either

  • CatalogMapper should operate on a non-Hotel-specific Address,

  • or, if Hotel.Address is somehow special w.r.t GeoCodes then Hotel.Address should be able to map itself to a GeoCode without a CatalogMapper.


With clarifications from the comments, I'd like to propose the following. I don't know C# so I'll write this as C++. I hope it translates

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 &);
};

Whenever I see a class called a "Manager", if it's not an object which creates, owns, and controls access to other objects, then it's warning alarm that we're in the Kingdom Of Nouns. Objects can manage themselves. You should only have a separate class for the logic if that logic reaches beyond the realm of a single class.

~没有更多了~
我们使用 Cookies 和其他技术来定制您的体验包括您的登录状态等。通过阅读我们的 隐私政策 了解更多相关信息。 单击 接受 或继续使用网站,即表示您同意使用 Cookies 和您的相关数据。
原文