[mythtv] [PATCH] Display Resolution switching

[Isaac Richards]

On Sunday 01 August 2004 02:20 am, John Patrick Poet wrote:
> >>>- Why is the DisplayRes class a public member of the context?
> >>
> >> I made it a member of MythContext because that is the primary place it
> >>is used, and other "classes" would have easy access to it.
> >
> >It's not a question of being a member, it's a question of it being a
> > public member.  It _should_ be private.
>
> Why?

I think it's poor design to have a non-pure data class have any public 
variables without accessors.  Regardless, it doesn't match anything else in 
MythContext either.

> >I'm not talking about the data of the DisplayRes class, I'm talking about
> >having a member variable be visible in the public MythContext for
> > absolutely no reason.
>
> Why?  How does a layer of abstraction help?

Why bother using access levels at all?  Hell, why bother using C++ if you're 
just going to make everything public to the world?

> >'if' without a space after it is horribly annoying.  'if' is not a
> > function. The displayres class is full of tabs as well.
>
> Another personal preference.  It does not bug me the way it bugs you.
> However, this is your baby, and I will do my best to do it your way.
>
> Believe it or not, 4 space indents actually bugs me.  I like to have
> option of nesting deeply when necessary.  I personally use 2 space
> indents so the code does not become a mess of line breaks.

If you're indenting that deeply, you should start a new function.  Code 
quickly gets unreadable and unmaintanable otherwise.

> When I went in to do a search/replace of "if(" to "if (" I found a lot 
> of instances in code that I had not written, so it has slipped by 
> before.  Sorry I forgot to "untabify" the DisplayRes code.

'Course it's slipped by before.  Doesn't mean I won't reject new patches if I 
notice things I don't like, though.

Isaac