better Object Oriented scheme

[EhsanMarufi]

Changed the private access modifiers to protected to enforce the OOP practices. By changing the fields' access modifiers to protected, one can customize the implementation while still utilizing the same interface.

[PrimaryFeather]

Thanks a lot for the pull request!

Throughout the framework, you'll find that I use "protected" only rarely. That's not because I don't like it (it is, as you say, an important part of OOP), but because a protected API is for me (as the framework developer) just as impossible to change as a public API. To put it differently: when I make a method or member protected, I'm promising the user that I won't change it later (or only in very critical circumstances), in order to not break any code. From that point of view, it's not different from a public API, really.

So I'm using protected only to support specific use-cases, not by default. Which leads me to the question: what exactly do you want to achieve in your case? What do you want to do with the TextureAtlas class that's not possible now?

[b005t3r]

Just a small clarification :)

Making every member protected is not an OOP practice, not a good one at least. As Daniel explained, you don't always want to expose all internals of a class, as this could create code (e.g. extensions) which depends on how this class was actually implemented. It's called "implementation inheritance" and in general is not a good idea. What you want is "interface inheritance" - a subclass has to rely on specific set of members only (mostly public members). The later is generally better, because no matter how the base class implementation changes, as long as its public interface does not change, your subclass should work as expected as well.

[EhsanMarufi]

Thank you for your comments :)

I agree with @b005t3r not to make every field's access modifier to be protected, the philosophy of the keyword is to encourage the OOP design, not to expose all the internals of a class. Actually, I've developed some APIs for dynamic atlas generation which utilize advanced rectangular bin packing (RBP) algorithms that I've developed alongside. When you're adding some new properties to the "sub-textures", you will need an extended SubTexture (in my case, I've added the most desirable feature of the "pivot point".) In the current TextureAtlas interface, the addRegion() method only adds new instances of the SubTexture class, leaving the developer no luck to insert the extended SubTexture. Considering the "interface inheritance" concept that @b005t3r mentioned and your coding prospectives, there should've been some method like addSubTexture(name:String, subTex:SubTexture):void in the interface (the equivalent concept to the current addRegion() method) to meet "the needs of tomorrow!" Regarding the parseBool() utility method; when you, for example, override the method of parseAtlasXml(), you will definitely be in need of such a method (i.e. the parseBool()), if it was available, you would "reuse" the available method avoiding "the wheel re-invention".

To preserve your coding prospectives in your truly great work, I've updated the "pull request" and added the missed method to the interface:

/** Adds a named region for an instance of SubTexture or an instance of its sub-classes.*/
public function addSubTexture(name:String, subTexture:SubTexture):void
{
    _subTextures[name] = subTexture;
    _subTextureNames = null;
}

So, in this "point of view", The field of _subTextureNames is absolutely an implementation detail and needs to stay private. So does the _atlasTexture field (although it's just a "reference" filed and thus makes a suitable candidate for being protected, and even a very slight performance gain by avoiding the "get" function invocation overhead).

[PrimaryFeather]

Yes, that's perfect! This community is just amazing. :smile:

@EhsanMarufi, now you've nailed it. With that change, the use case you had in mind is supported, and we're not giving away any implementation details. Exactly what I wanted!

I merged your updated pull request, and then made two small modifications:

• The method addRegion is now internally calling the new addSubTexture, to avoid code duplication.
• I moved the parseBool method to the StringUtil class — a much better fit, anyway, because that way, it's available for all framework users. It is a handy utility method, after all.

Thanks to both of you! :smile:

[EhsanMarufi]

Just amazing and glorious as ever :) Thank you :)