[haiku-commits] Re: haiku: hrev52055 - src/kits/media headers/os/media

  • From: Dario Casalinuovo <b.vitruvio@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Thu, 26 Jul 2018 14:48:27 +0200

Hi,

On Thu, Jul 26, 2018 at 2:36 PM Stephan Aßmus <superstippi@xxxxxx> wrote:
[...]

... and Clear() are incorrect, since they don't do anything with the
previous contents of meta_data and meta_data_area. They would actually
need to do what the destructor does, but before memset(). (And this is
actually why the manual memset() in the code that was replaced to use
Clear() was also buggy, media_format actually has a non-trivial
destructor). This is an example of producing bugs by duplicating code,
even if it is only two lines. Always refactor into methods in order to
never duplicate code. Of course, when Clear() is called from within the
constructor, and also from the copy-constructor, both meta_data and
meta_data_area would not be initialized, so the contructor and
copy-constructor need to do that /before/ calling Clear().


Don't you think that if we used the standard ctor and dtor, and operators,
as I am proposing, instead of introducing methods, this issue wouldn't be
there?

Doesn't that suggest you that the more you don't follow the rules, things
can just get worse?


Note, this does not touch on the topic whether Clear() should stay
public API or should become a private method, or wether it should be
implemented via memset() or not. As is, the code is just definitely
incorrect in this regard.


I agree. Tomorrow I will also add an Equals() function because I think this
is more readable than the operators provided by the C++ standard.

PS: this is humor not an attempt to attack you.

--
Saluti,
Dario

Other related posts: