Model.load(...)

In the spirit of an explorable API, I think the Model.load() methods should be turned into constructors.



There’s nothing legal you can do with a Model that’s not yet loaded (calling getAnimationController() before load(), for example, causes a NullPointerException), so loading in a method adds nothing but the possibility for programmer error - calling it at the wrong time, or calling it twice, as in:


MilkshapeASCIIModel model1 = new MilkshapeASCIIModel();
MilkshapeASCIIModel model2 = new MilkshapeASCIIModel();
model1.load("model1.txt");
model1.load("model2.txt");
scene.addChild( model1 );
scene.addChild( model2 );



There's some aesthetic value in unifying the signature of the load methods across the various types of Model, but there's not much practical advantage since nobody will ever call the abstract method without calling a concrete constructor immediately beforehand (similarly, it's not safe to call Model.load( filename) without knowing the concrete class).

There's a nice example of the sort of explorability I'm referring to in Hibernate's transaction idiom:

Transaction transaction = session.beginTransaction();
// do work
transaction.commit();



It's somewhat unexpected if you're looking for transaction.begin(), but having an API like this means that it's impossible to commit before starting a transaction, which reduces the chance for error.

There are two constructors:



MilkshapeASCIIModel(String name) and MilkshapeASCIIModel(String name, String filename).



The second calls load automatically. Perhaps I should just delete the first constructor, because, as you said, the model is unusable until load is called.

Yes, that would be great. The only other suggestion I have would be adding another constructor:



MilkshapeASCIIModel(String name, String filename, String textureDirectory)



…and making the load(…) methods private (or perhaps @deprecated, as a first step).

Why not have something like:



Model myModel = ModelLoader.load("myModel.txt”, ModelLoader.guess);
Model realmd2 = ModelLoader.load("reallyamd2.txt”, "md2");



The method could guess model format from the extension and the programmer would not need to know the format. If the programmer wants to specify the format or the extension is incorrect the programmer could just change the last parameter.

Not a bad idea. I’ll definately think about that.