sphair / ClanLib

ClanLib is a cross platform C++ toolkit library.
Other
344 stars 76 forks source link

Rejection of imp-> #102

Closed ArtHome12 closed 6 years ago

ArtHome12 commented 6 years ago

In the clanlib's architecture is widely practiced that behind the class there is an implementation. This speeds up the compilation of the library itself at the expense of the optimality of the final code. Is it generally justified for libraries?

dpjudas commented 6 years ago

At the time it seemed like a good idea. There are only a few alternatives to this approach, each with their pros and cons:

1) You could take the API classes and turn them into interfaces. All API functions are made virtual, and then the impl class inherits from the API class. Cost is that you can't have the shared_ptr as a member of the class with this approach. Take a look at the UICore library on my github page to see how such a solution looks like.

2) You could take all the private functions in the impl class and make them private in the API class. This has the disadvantage that the API header may have to include implementation specific headers, which may slow down compile time for programs using the library.

3) You could remove the impl completely. Same disadvantage as #2, but slightly faster as it won't have to access the impl to access private data. Now everything is in the open, for better or worse.

I generally write my libraries nowadays using method 1 listed here, as that still allows me to hide the implementation. ClanLib hasn't been moved over this approach as it would mean every program using it would have to be heavily refactored.

ArtHome12 commented 6 years ago

Thanks for information. So, ClanLib is built using two techniques:

  1. Using imp-> to hide the full interface (effects - acceleration of compilation and deceleration of execution and debug).
  2. Different paths for headers and sources #101.

I may be mistaken, but it seems that this approach is inherited from closed source systems. If this is so, and it makes no sense for an open source project, can we go on to method 3 (not everywhere, but only where implementation is trivial)?

dpjudas commented 6 years ago

I didn't have close source systems in mind when I designed it this way. Rather, it was an attempt to leave implementation specific details out of the public header files.

Personally, I'm not going to try change this part of ClanLib's design. I'll be perfectly honest with you, at this point ClanLib only interests me historically as an illustration how I did library design in the 00's. It was great fun to code, especially my interactions with Rombust, Sphair and Mrfun, but I've since then moved on.

I wouldn't mind creating a "ClanLib Next", but the general attitude in the indie gaming world seems to be that you either need to use Unreal Engine or Unity and any other approach is folly. As long as that prevails there's little point as far as I'm concerned. I do play around with some game related stuff on a personal level, but it isn't based directly on ClanLib and it isn't at a level where I want to make it an open source project - at least not yet.

So basically what I'm saying here is that you're more than welcome to improve ClanLib with what you think it needs, but I won't be assisting in it. If you send PR's I'll evaluate them and merge if I don't see anything technically wrong with them. For larger fundamental changes it might be better to create a fork as I can't guarantee I'll approve of it and it would suck if you spent the work just to see me reject the pull request.

rombust commented 6 years ago

Historically, the main reason the implementation is hidden was to assist with library binary compatibility when it was dynamically linked (e.g. dll's). We could patch the implementation, without the requirement of recompiling each application.

rombust commented 6 years ago

My thoughts are the same as dpjudas.

I use ClanLib at work, however the code base has now diverged from github containing my specific requirements.

ArtHome12 commented 6 years ago

I do not want to make a fork and lose your experience for advices as long as I can keep backward compatibility. Thanks :)

There is awaiting PR #100 and then continue.