[mythtv] frequent PlayBoxTransparency setting DB lookup in
myth at colin.guthr.ie
Tue Aug 17 15:37:50 EDT 2004
Donavan_Stanley at ham.honda.com wrote:
> Again: Replying from work with a crappy client. I apologize.
The text seems to get through, so that's the main thing ;)
> Pretty much everything I'm doing is in MythVideo, except for a minor change
> to the UIImageType so that I can get the filename of the image it currently
> has loaded. I'm changing VideoDialog to be MythDialog derrived instead of
> MythThemeDialog. With that in and the changes I made to how the cover art
> gets handled in places things should be much snappier when drawing.
Cool. I've done a lot of C++ stuff before but never used QT before, or
familiarised myself in too great depth with the Myth framework as the
docs are more or less "read the source" unless I'm mistaken....
> I did look at your patch and had planned on applying at least part of it.
> There was one line that seemed to be wrong, there was a check for the
> failure of a query that used "rowsaffected == 1" when it seemed like it
> should have used "rowsaffected < 1".
Ohh, I think the if statement is all wrong in that line. I switched the
logic around at one point to return and bail rather than nest loads of
blocks/braces and forgot to change the && to || and the == to !=.
Whoops. Sorry. :-[
> I also wondered why you tightened the checks for the queries from ">0" to
> "==1". It shouldn't really impact things one way or another, I just
> didn't understand the reasoning behind it.
Yeah, sorry, I should have left these. Just me being paranoid. I do a
lot of SQL stuff at work and always do a check to see if I get 1 row
when I expect 1 row. I guess there is no real need for it, just me being
anal no doubt!
It's part of my job sometimes to review code and change things to match
our coding standards so sometimes I instinctivly change things like this
to be more in line with how our coding guidelines are written. It's
almost second nature, but I shall try and refrain (and try to get used
to 4 spaces indents too!).
> Oh and, there's no need to use QString.sprintf when doing a simple
> assignment. There's no sense in parsing the string for replacement chars
> if we know there aren't any. Actually is there a real need to create a
> QString out of that literal string at all?
That is a good point, too much changing/copy of existing code and not
enough thinking by me on that part, the sprintf is totally unnessary!
I wont bother making another patch unless you would like me to, seeing
as it has just been used to point out a couple of weirdnesses (albeit
with it's own weirdnesses added for good measure!) in the metadata code.
Hopefully a couple of the changes are useful.
| Colin Guthrie |
| myth at colin.guthr.ie |
| http://colin.guthr.ie/ |
I feel like I'm diagonally parked in a parallel universe.
More information about the mythtv-dev