Advertisement

Create many factory methods

Started by May 26, 2015 11:07 PM
5 comments, last by frob 9 years, 7 months ago

I am right now trying to bind my C++ code to my Java code and cross language programming has lots of benefits but sadly also disadvantages.

I.e. this C++ factory method


Letter* Alphabet::CreateLetter(const int* type) {
     if( type == 1 )
          return A;
     if( type == 2 )
          return B;
}

Doesn't translate well into Java because Java only knows that it gets an object of the type "Letter" but can't predict if it is one of the derived classes "A" or "B" and casting those classes on the Java side fails.

So my personal workaround to this problem is to create multiple factory methods like

A* CreateLetterA();

when Java calls this method it knows it gets an object of Type "A"

However this involves that I am

1. Create a factory method for every single derived class in the .h and .cpp file

2. Include all 36 derived classes into the .cpp file (That is a veeery long list)

Any ideas how I could improve this before I start writing everything down and later realize this could have been maybe achieved a bit easier? ... Maybe?

You should be coding to an interface, not an implementation. Your java code shouldn't care whether it's a "A", "B", or a "Z", only that it is a Letter.

Advertisement


You should be coding to an interface, not an implementation. Your java code shouldn't care whether it's a "A", "B", or a "Z", only that it is a Letter.

Your reply neither helps in any way me nor answeres my question tbh, sadly.

Sadly it does and it needs to.

It is one factory method that delivers different objects back I want to work with as those objects extend the baseclass with additional methods and functionality. If I want to use them I need to cast them as all of those objects can only be instatiated through this single factory method.

The codebase is from Google and that is also written down in their comments that I have to cast the different classes I get from the factory method to use them. The baseclass is not interesting for me.

And if Google says it works like this, it works like this! (:D)

I don't like it either but I can't imagine any other way either as the factory method basically gives me a class of type "Animal" but it can be any animal and if you call the factory class to create a horse and want to interact with it to do horse things I need to cast it as every animal is so hugely different from each other. The classes the Google code gives me back is exactly like this.

I did now create 23 different factory methods in the class like this


Letter* Alphabet::CreateLetterA() {
     return (Letter*)CreateLetter( 1 );
}

I am not happy with it as there is now a big block of this bloat code in my .cpp and .h and Java files, plus all those forward declarations...

Upcasting from a base class to a derived class is a warning sign of a potentially bad design. Normally, the interface methods would make calls to any additional methods to provide extra functionality, but would not require the user of the API to call those methods directly.

If you are set on using the this particular library, then you're pretty much stuck creating factory methods for whatever derived classes you plan to use.

I'm sorry if you didn't find my first answer helpful. I though you were interfacing with your own C++ code, as you stated in your first post. In that case, my answer was intended to give you an opportunity to rethink the design of the C++ API and possibly eliminate the need for upcasting.

Oh! After reading your reply I immeadiatly rushed to the top and noticed that I wrote "my C++ code" accidentially.

My bad ohmy.png

Thanks for the advice!

I mean I could rearrange the whole Google project but then I could just write it by myself as well biggrin.png

Thanks

It is called the Dependency Inversion Principle, and should be followed in almost all systems.

Systems should (almost) never need to know about the concrete class involved. All components should be written against the abstract base class that defines the interface.

As an example from another similar thread recently, there was a bit of confusion regarding base classes.


// Start with a pointer to a base class
shared_ptr<AvatarBase> avatar = ... 

// BAD, you are relying on concrete type details. Breaks if you add a new subclass.
if( Wizard* wizard = dynamic_cast<Wizard*>(avatar.get()) ) { /* do Wizard stuff */ }
else if( Warrior* warrior = dynamic_cast<Warrior*>(avatar.get()) { /* do Warrior stuff */ }

// BAD, you are relying on concrete implementations details. 
// In addition to breaking on new subclasses, also breaks if you derive from any specified class.
if( avatar->subclassKey() == Wizard::subclassKey) { /* do Wizard stuff */ }
else if( avatar->subclassKey() == Warrior::subclassKey) { /* do Warrior stuff */ }

// GOOD. This is how inheritance is supposed to work.  
avatar->DoTheThing();

Every time you write code that depends on knowledge of the concrete class, you are almost certainly writing a bug.

Specifying a concrete type means you cannot later extend the class to do something different without going back to all that code and adding the new extended type.

Specifying a concrete type means that if you need to make a new subclass you need to revisit all those areas of code and add your new subclass.

If other systems use your code, then you need to modify those systems as well. If your code is shared with your main engine and also between ten or fifteen different tools, you need to modify and rebuild and test all of those tools in addition to modifying your code base.

Specify the interface and write all code against that. In C++ that is an abstract base class. In Java that is an interface.


Letter* Alphabet::CreateLetter(const int* type) {

C++ factories should return either a shared_ptr or unique_ptr based on ownership mechanics in the code.

Otherwise it is easy to have code accidentally create an object and leak it.

Java's memory model does that automatically due to garbage collection.

Advertisement

Oh, as for why it is "almost never", there are some extremely rare cases where it is necessary.

As one example, if you have a specialized version that comes from a library you cannot modify --- perhaps it comes from an external vendor and you do not have a source code license -- and you discover a bug in that one specific subclass, then it can be necessary to adjust a value in a specific concrete class.

So you might have a bunch of Direct3D calls, but then you have some specific function where you found a bug in a card/driver combination. Then you would normally use your D3DSurface or whatever, but in some extremely rare case you have specialized that if the driver is a specific concrete implementation and other values match, then you need to take a different action.

In practice it is extremely rare to *need* to look at the concrete class. If your code looks at the concrete class you are almost certainly writing a bug and probably have a design flaw.

This topic is closed to new replies.

Advertisement