Quote:myModel& monster = Model( "BigMonster" );

This is not legal C++. Temporaries may not bind to non-const references.

Quote:Original post by loufoque

Quote:myModel& monster = Model( "BigMonster" );

This is not legal C++. Temporaries may not bind to non-const references.

Consider it pseudo-code. I just wrote it on the spot to illustrate a point, I didn't think much about correctness.

Quote:Original post by Zahlman
Hold on. Let's take a step back first.

Why do you have managers?

What do they do?

I usually use managers to control centralized resources. For example, the Filesystem class handles all disk loading and caching, the Material class parses material config files and outputs material data structures, the Gui class maintains the UI state and outputs to the renderer, etc.

Most of these probably could be decentralized, but I would probably have to use a lot more static member variables to share the central data being used. Also, having a class internally create instances of all these big classes in order to use them seems a lot less modular than actually passing in existing references to them.

Quote:Original post by Antheus
And voila, the spaghetti are untangled.

To load a model, you call your Asset class instance, and use loadModelFromFile utility method.

Thank you for your detailed comment, I am still re-reading it to fully get it. However, if you ignore all of the logical restructuring you did, aren't you just trading what dependency needs to be passed around? Rather than creating a Model object, and passing the Filesystem and Material references to it, you're being passed a central Assets reference and loading the model from there.

[Edited by - Nairou on May 30, 2009 11:21:10 AM]

Quote:Original post by Nairou
However, if you ignore all of the logical restructuring you did, aren't you just trading what dependency needs to be passed around?

The sky is blue. A depends on B. These is the world we live in, the cards we are dealt.

Yes, I am trading dependencies and shuffling things around, but I let dependencies guide me. The fact that mesh will somehow need to be loaded from file is a given.

What I try to do is minimize the coupling until it boils down to bare minimum. I would likely factor out both filesystemManager as well as materialManager, but I don't know what they do, so I can't.

There is actually another step possible that I didn't perform before.

class Assets {  FileSystemManager filesystemManager;  MaterialManager materialManager;  Assets(FileSystem fs) {    filesystemManager = fs;    materialManager = new MaterialManager(fs);  }  Model loadModelFromFile(string s) {    // implemented above  }};

Quote:Rather than creating a Model object, and passing the Filesystem and Material references to it, you're being passed a central Assets reference and loading the model from there.

I'm not passing around anything. My entire application looks like this:

int main() {  FileSystemManager fs = new FileSystemManager();  Assets assets = new Assets(fs);  Model m1 = assets.loadModelFromFile("mesh1");};

This is as simple as it gets.

This is my world, there is absolutely nothing else, hence according to YAGNI I don't implement it. The code above is a result of refactoring - I didn't plan for it. I'm not saying "I am right" when code disagrees with me. I could very well refactor to do this:

int main() {  FileSystemManager fs = new FileSystemManager();  Source s = new ModelLoaderSource(fs, materialManager);  Model m = new Model(s);}

Same thing, but since I will need to create more than one model, I might as well wrap it into a factory. The important thing is, the design is highly decoupled, and I can now perform changes like these with minimal impact.

It's the "once, twice, refactor" rule. I could start with later implementation, and as soon as I'd to create ModelLoaderSource in another place in code, I would refactor.

IOC is incredibly convenient for agile development since it allows you to focus on tiny details and mix-and-match them as needed. Since everything is coupled via interfaces and references only, I can trivially develop and test each part in complete isolation, even before other systems exist.

And this also solves:

Quote:the desire to still provide clean APIs for the rest of the code to use.

The above is clean API. User uses Assets instance to create meshes. It doesn't matter where it comes from. You can provide an existing instance, a global (not singleton, since there can be multiple), or they can create one on their own.

Assets::loadModelFromFile(string) is as clean as it gets. The user then simply keeps as many copies of assets as reasonable, usually one per scene.

But the biggest problem with your API being cluttered comes from putting too much functionality into Mesh. It needs to know how to access file system, read data, load and store materials - in addition to what it really is - a data holder, a POD.

Consider something else - why isn't Model being created from what it really is: a list of vertices and triangles with additional texture data. Something like:

Model::Model(Vertex v[], int nVert, Tri t[], int nTri, Texture * tx[], nTex);

Because this is really what a model is. Loading is orthogonal functionality which almost all serialization tools implement externally, but often expose it either via member functions or in constructor.

This change completely decouples model class from everything else, even from standard containers, meaning it's often suitable for C-interoperability. I can then use it with standard containers, with stack-allocated data (convenient for loading to avoid allocations since model file often specifies total number of elements), or something else.

Thanks, Antheus, you've given me a lot to think about. I must admit I'm still learning to follow YAGNI, I have a tendency to plan ahead rather than just write what I need now.

Quote:What I try to do is minimize the coupling until it boils down to bare minimum. I would likely factor out both filesystemManager as well as materialManager, but I don't know what they do, so I can't.

I can see how my Material class could be factored out (as long as you provide a Filesystem reference, it consists of little more than a loader function that outputs complex structs), but I'm curious how you would factor out the Filesystem class (as an example).

Granted, it too consists of little more than a loader function. However, it does contain a private std::map, where it caches the data it loads. That map still needs to exist somewhere, so the only factoring I can think of is to make it static and turn the Filesystem into a purely static class.

But that leads to the whole question of why I don't turn a lot of these classes into static classes, why static classes might be "bad", etc.

How would you handle this?

Quote:Original post by Nairou
I can see how my Material class could be factored out (as long as you provide a Filesystem reference, it consists of little more than a loader function that outputs complex structs), but I'm curious how you would factor out the Filesystem class (as an example).

Java and C# both define Readers and Streams. These have proven to be adequate abstractions for such tasks. They make clear separation of content provider and content handler. While in C++ it would be implemented slightly differently, boost::serialization defines a common way to approach this problem.

The key concept here is that Reader is capable of instantiating a concrete object (Model/Mesh/Texture), so that is who must be passed the references to relevant user-defined objects (Factory, cache).

Quote:Granted, it too consists of little more than a loader function. However, it does contain a private std::map, where it caches the data it loads.

So it's a cache. Why not call it as such.

Quote:That map still needs to exist somewhere, so the only factoring I can think of is to make it static and turn the Filesystem into a purely static class.

There is no such thing as static class in C++. An instance of a class can be allocated statically in global context. This is technical implementation detail, not design issue.

Static allocation is commonly abused to define a specific and centrally accessible instance. This requirement is rarely needed, and less often desired specifically in C++, due to initialization issues.

Quote:How would you handle this?

Like I demonstrated above. User allocates the instances. If they choose so, they can make them static.

Design doesn't care about how something is allocated, since references are passed as needed.

If you want a clean API, I would suggest to stop mixing Java (or is that C#?) and C++, and even not to code Java-style at all in C++.

Quote:Original post by loufoque
If you want a clean API, I would suggest to stop mixing Java (or is that C#?) and C++, and even not to code Java-style at all in C++.

Err, huh? This is all C++. And pseudo-code. Can you explain?

Quote:

int main() {  FileSystemManager fs = new FileSystemManager();  Assets assets = new Assets(fs);  Model m1 = assets.loadModelFromFile("mesh1");};

This is obviously not C++. new FileSystemManager() would return a FileSystemManager*, not a FileSystemManager.

The same can be said for all your code.

The good practices of C++ are to use RAII, not pointers galore.

Whether a given object is a pointer type or not isn't really relevant to this discussion.