search

CasparCG / server

CasparCG Server is a Windows and Linux software used to play out professional graphics, audio and video to multiple outputs. It has been in 24/7 broadcast production since 2006. Ready-to-use downloads are available under the Releases tab https://casparcg.com.
GNU General Public License v3.0
905 stars 268 forks source link

Bug: Use of pimpl pattern #1569

Open dimitry-ishenko opened 1 week ago

dimitry-ishenko commented 1 week ago

Observed Behavior

Every time peek into the Server innards, it makes me cringe a little. 😄 I think I've asked this before, but I don't remember what the answer was...

Why do we use pimpl pattern for the Server?

I understand when you have a library and you want to (1) hide implementation details from users, and/or (2) keep the ABI constant. That's when pimpl design comes in handy. Well, also (3) it helps reduce compilation time.

Expected behaviour

Neither (1) nor (2) applies to the Server though. Looking at the amount of headers included in the implementation files, I am not so sure (3) does very much either. Plus, there are other ways to mitigate compilation times.

Downsides of pimpl are pretty substantial:

I would expect the Server to use the usual OO pattern with no pimples (pun).

Steps to reproduce

  1. Open the Server code in your favorite editor.
  2. Observe the pimpl pattern being used everywhere.
  3. Cringe a little 😉

Environment

Julusian commented 1 week ago

https://github.com/CasparCG/server/issues/1327

I don't have particularly strong feelings on this, other than I do find working with the boilerplate for methods rather tedious. And I don't feel confident enough to change what works and is already there

I do wonder how much more removing that would cause large recompiles.. It does 'leak' some implementation details to consumers of the headers, but in reality how often will they change without changes to the public methods..

dimitry-ishenko commented 1 week ago

Good on finding that... I guess I should try using the search function next time. 😅

dimitry-ishenko commented 1 week ago

1327

I don't have particularly strong feelings on this, other than I do find working with the boilerplate for methods rather tedious. And I don't feel confident enough to change what works and is already there

I do wonder how much more removing that would cause large recompiles.. It does 'leak' some implementation details to consumers of the headers, but in reality how often will they change without changes to the public methods..

I will try to pick a small part (eg, const and mutable frame) and see what I can come up with... at some point...