1

I am facing a design challenge with my classes and being new to C++ and OOP I am hoping I can get some advice on how this should be handled.

The situation is that for example I have 2 (and in the future more) variations of a Customer class that I want to treat as a single member variable of the Project class. The different Customer classes do not share much in common because they are intended to be used to store data pulled from different APIs.

The code for example purposes would be as follows (ignoring lack of constructors/destructors etc. for the purposes of discussion)

class Project
{
   Customer customer;  //???
}

class Customer1
{
   std::string name;
   std::string address;
}

class Customer2
{
   std::string name;
   std::string primary_contact
   bool approved;
}

My initial idea was to use polymorphism and create an abstract base class AbstractCustomer and make Customer1 and Customer2 derive from this base class. This way I would only have 1 member variable of AbstractCustomer* customer inside Project and be able to effectively build in logic to dynamically determine which Customer subclass is needed.

My dumb option was to simply have separate member variables e.g. Customer1* customer1 & Customer2* customer2 which are pointers and then effectively use a nullptr to indicate which ones are not in use. This however doesn't feel right and of course when more Customer class variations come up then I would need to keep adding more member variables to the Project class.

note: I am using these classes more for POD purposes and don't envisage having many methods, which is why I am unsure about using interfaces/abstract classes since it seems that's where the benefits of it lie.

Any advice/guidance is appreciated.

SimStil
  • 59
  • 1
  • 8
  • 3
    "_My initial idea was to use polymorphism and create an abstract base class_" - That sounds like a good start. "_...and be able to effectively build in logic to dynamically determine which `Customer` subclass is needed_" - ideally, not. If you use the same interface for all subclasses, you wouldn't need to do that manually. – Ted Lyngmo Aug 18 '21 at 12:21
  • If you open your C++ the textbook to the chapter that explains how to use `std::variant` you will find more information about how to do this. Note that this is a fairly advanced topic, and you may need to review some more basic subject matter first, and understand the various rules related to proper use of advanced C++ templates. You will also need a modern C++ compiler that supports at least C++17. Are you familiar with `std::variant`? – Sam Varshavchik Aug 18 '21 at 12:21
  • 2
    Personally, I would just have one customer class that has all of the felids you needs. To me it doesn't really make sense to have different types of customers. If you want to classify them differently, like premium, pro, regular, ect., then I would add a field to the class that you can set for which type it is. – NathanOliver Aug 18 '21 at 12:21
  • there are many options and which is the right one depends on details. Another one is when a `Project` holding a `Customer1` and a `Project` holding a `Customer2` don't need to be the same type then you can make `Project` a class tempalte and then instantiate `Project` or `Project`. – 463035818_is_not_an_ai Aug 18 '21 at 12:25
  • @NathanOliver I did consider this however, it gets more complicated where each type of customer will also contain member variables of say an 'Address' type, and each will have its own different member variables, so then I would need to repeat what I've done with the customer class and have a catch all class to server as 1 variable. – SimStil Aug 18 '21 at 12:26
  • @SamVarshavchik I have heard of this but never explored it, luckily our application does support C++17 so I will look into it – SimStil Aug 18 '21 at 12:28
  • @463035818_is_not_a_number this might be worth looking into, although I have never created templates before. Essentially a project will be based around the particular API we are trying to link to which will be known at time of creation. There will therefore be multiple class similar to ```Customer``` such as ```OrderDetails``` that can all be different depending on the API, although all the same combination (if that makes sense?) i.e. it will always be ```Customer1``` and ```OrderDetails1``` and ```Customer2``` and ```OrderDetails2``` – SimStil Aug 18 '21 at 12:32

1 Answers1

0

I think the major goal is to keep the complexities of the variant customers out of Project. Customer is probably going to be gross, but if we keep all of that complexity in one place, then we're probably ok.

Option 1)

If the fields in the customer variants are all individually consistent, then you could probably use a template or a variant as others have mentioned in the comments.

What I mean by this is that each field is never interpreted differently based on what base type is. Name is always used the same way, that's probably safe example. Primary contact is always a primary contact, it doesn't mean a billing address for a BillingCustomer or a shipping address for a ShippingCustomer.

Then I would use type traits to return empty data if it's missing for that customer base type. Project will have to assume the complexity of knowing what to do with a customer that doesn't have a primary contact.

I won't get into details of type traits and SFINAE, but you can find plenty of information in various questions and I'm sure others.

template<typename T>
Customer {
public:
  // invariant field, always available
  std::string getName() {return customerData.name;}

  // variant field, not always there
  std::optional<std::string> getPrimaryContact() {
    if constexpr (has_field<T,&T::primary_contact>::value) {
      return customerData.primary_contact;
    } else {
      return std::nullopt;
    }
  }
private:
  // Implementing this with std::variant would probably make Project
  //   cleaner.  It's a lot more boilerplate to illustrate the above
  //   concept though.
  T customerData;
};

Based on my experience of how businesses work, this option's criteria are very optimistic. Most businesses will have really dirty modeling that will make this option much less attractive.

Option 2)

More generally, business rules are more complex and you have to do interpretation of field values based what the different types are.

In this case, I'd just make you decision making explicit. This is going to feel like a ton of boring boilerplate and it is. It's not dynamic logic though, the code is inert. Bugs are identified quickly and resolved quickly.

This code is simple.

class Customer {
public:
  std::string getName();

  // Assuming there are cases where most optional fields won't exist
  std::optional<std::string> getPrimaryContact() {
    if (customer1) {
      return customer1->address;
    } else if (customer2) {
      return customer2->primary_contact;
    }
    // Eventually this will be relevant
    return std::nullopt;
  }
private:
  std::optional<Customer1> customer1;
  std::optional<Customer2> customer2;
};

With both options, you expect Project to have to manage missing data.

void Project::SendMailToCustomer(const Mail& mail) {
  auto primaryContact = customer.getPrimaryContact();
  if (primaryContact) {
    SendMailToAddress(mail, *primaryContact);
  } else {
    std::cout 
      << "Did not send mail to customer with empty primary contact: " 
      << customer.getName() 
      << std::endl;
  }
}
Tom Kerr
  • 10,444
  • 2
  • 30
  • 46