[mythtv] [PATCH] DVB support

[Ben Bucksch]

  Isaac Richards wrote:

> On Friday 02 May 2003 12:25 pm, Ben Bucksch wrote:
>
>> I didn't understand you that way. Should I just *copy* all your code
>> which is in mpegrecorder after my patch (for seeking etc.), 320 lines in
>> 24 functions, although it's identical in 3 files, which all only have
>> 150 lines and 6 functions or less right now? 
>
> Yes. 

*sigh*

I usually don't even copy 20 lines of code, because it will lead to a 
maintainance hell. Somebody fixes a bug in one instance, but not the 
other, because he doesn't realize the other. Somebody else fixes a bug 
in the other one, but not the first. A year later, somebody else notices 
the differences, but doesn't know *why* they differ (there could be an 
obscure reason...), so he doesn't merge them. Stuff like that *does* 
happen.

Here, we're talking about over *300* lines, and you want me to copy it 
*several* times, although I have a solution that works just as well and 
doesn't copy at all. It would hurt me, but I would take the time and 
break what is IMHO a clean and working design. But what if you fix a bug 
or otherwise improve the general MPEG code in MpegRecorder, e.g. the 
seeking stuff? Would you also always copy your changes to all the other 
classes? With the current patch, your new changes immediately apply to 
all derived classes.

Can you maybe tell me *why* you want me to copy that code?

>> That means that I have to rename *all* the references to |Channel| in 
>> all other files. Also, I find it confusing to have the 
>> analog-specific part having a generic name like Channel.
>
> Yup, that's what it means.  Trying to match the existing abstraction 
> layers -- recorderbase and decoderbase. 

But at least the analog part isn't called "Decoder" and "Recorder". I 
thought that other contributors would be confused about generic names 
like "MpegRecorder" and "Channel" for the analog-specific part, when 
almost every other class hierarchy I've seen reserves generic names for 
the base classes (Animal, QWidget etc.), but I guess I could live with it.

> But it's only 2 files, since you should also be taking out the stream* 
> stuff, as you said it's not fit to commit anyway. 

It was an example for more like it which may exist in the future, e.g. 
HDTV.

>> Possible, havn't noticed that, will check.   
>
> Since you made it pass that int to the channel object, it's there =) 

hah! I can't be *that* dumb, can I? *lol*

> Hmm..  I think the newstruct.pro stuff should get included in the main 
> settings.pro file. 

Really? OK...

> I have to maintain this stuff, and live with the code.  You don't =) 

heh, yes, I understand and appreciate that and the implications, and I 
am also aware that you wrote the bulk of this code and it's your baby. I 
surely tried to deliver good and maintainable code to you :-). I 
unfortunately spent far more time than reasonable, and I also reserved 
some time to fix reasonable complaints during review, but it's hard to 
spend many hours to rework code into a state that I view as much worse 
than the current one, causing problems later.

Ben