1

I have a class structure like the on below.

    class Technology;
    class BasicTechnology : public Technology;
    class BasicChildTechnology : public BasicTechnology;
    class ConcreteChildTechnology1 : public BasicChildTechnology;//concretechildtech1
    class ConcreteChildTechnology2 : public BasicChildTechnology;//concretechildtech2
    class ConcreteChildTechnology3 : public BasicChildTechnology;//concretechildtech3
...
    class ConcreteChildTechnologyN : public BasicChildTechnology;//concretechildtechN

The ConcreteChildTechnologyN/3/2/1 has an isValid(String selector) method, which is as shown below,

public isValid(String selector){
 return "concretechildtech1".Equals(selector);
}

Now in the client code is,

Technology tech = getTechnologyObject("concretechildtech1"); //tech becomes an instance of ConcreteChildTechnology1

How should I implement getTechnologyObject() in this case?.

Thought of using the abstract factory pattern, but doubtful of that. or even create a Facade and create the concrete child based on the input argument? The problem is, only the ConcreteChildTechnology1 knows whether the input string (concretechildtech1) belongs to him or not; via isValid() method.

Again if I start to create N objects every time to check the validity, that will cause and overhead, because 1)the system is running in a very low memory environment like mobile and tablets, 2)the number of instance creation is high, 10-100 per minute.

May be make the isValid() a static inline method and create object based on the reply from child objects?

eyllanesc
  • 235,170
  • 19
  • 170
  • 241
Jimson James
  • 2,937
  • 6
  • 43
  • 78
  • 1
    `Technology tech = getTechnologyObject("concretechildtech1"); //tech becomes an instance of ConcreteChildTechnology1` Wellllll. Not quite. `tech` needs to be a pointer or a reference [or the object will be sliced](https://stackoverflow.com/questions/274626/what-is-object-slicing). – user4581301 Nov 29 '18 at 02:02
  • Is it just me or your code feels more like C# than C++? –  Nov 29 '18 at 02:52

2 Answers2

2

My understanding is that getTechnologyObject("string") is returning a smart reference/pointer like std::shared_ptr<BasicChildTechnology> based on a string. Inside that function there is a list of these tech objects and only that tech object knows if it is associated with that string.

The first problem is that string. Is it possible to convert it to an enumeration or some more precise data type earlier than now? That alone will make your system more reliable, and faster.

The second problem is the ownership of the match criteria. I imagine when the system was being designed that this felt natural. I would point out that this object does not have a single responsibility. It is both required to do whatever the Tech is, and required to match itself from some serialisation format. It may still make sense to leave the string inside that object (it might be a name) but the matching needs to be elevated out of the object, and into the search function getTechnologyObject("string").

Now regardless of if you have a string/numeric, the tech objects need a function virtual label_t label() (name it as you feel fit) that returns this identifier.

Thirdly your creating a new object each time. That is the factory pattern, but there are two choice on how to implement that. One is giving the power of cloning to each implementation and treat each instance as a prototype. The other is to create a related hierarchy of factories that build those tech objects.

If you go the prototype path also define a virtual std::shared_ptr<BasicChildTechnology> clone() const =0; in the Tech classes. Otherwise create a related TechnologyFactory class tree, or a Factory<T> template. The factory will need something like a label_t label() and a std::shared_ptr<BasicChildTechnology> build().

I'm going to pick prototype here.

Construct the lookup like:

std::map<label_t, std::shared_ptr<BasicChildTechnology>> lookup;
lookup.add(tech1->label(), tech1);
lookup.add(tech2->label(), tech2);
lookup.add(tech3->label(), tech3);

Then:

std::shared_ptr<BasicChildTechnology> getTechnologyObject(const label_t& label)
{
    return lookup[label]->clone();
}

And a Factory template here.

Construct the lookup like:

std::map<label_t, Factory<std::shared_ptr<BasicChildTechnology>>> lookup;
lookup.add(factory1->label(), factory1);
lookup.add(factory2->label(), factory2);
lookup.add(factory3->label(), factory3);

Then:

std::shared_ptr<BasicChildTechnology> getTechnologyObject(const label_t& label)
{
    return lookup[label]->build();
}

The lookup will execute in log(N) time, for both cases.

Kain0_0
  • 319
  • 2
  • 6
1

What you are trying to do has different solutions based on your exact implementation and what the child types actually do.

If the isValid() method never relies on non-static member variables, isValid() could be made static. Your getTechnologyObject() function could be written as:

Technology* getTechnologyObject(const std::string& _string)
{
  if(ConcreteChildTechnology1::isValid(_string)){
    return new ConcreteChildTechnology1(/* arguments go here */);
  } 
  /* follow with the rest */
}

As per user4581301's comment you can return a pointer to prevent object slicing.

It seems that your type hierarchy is blowing out in size. To reduce complexity and perhaps make the creation of objects easier, you could explore some form of composition instead of inheritance. This way a factory pattern would make more sense. Perhaps you could create a Technology object based off what is it supposed to do using a decorator pattern.

Bar Stool
  • 620
  • 3
  • 13