[Qt] TScrollBar patch
zeljko
zeljko at holobit.net
Tue May 29 10:07:43 CEST 2007
On Monday 28 May 2007 23:19, Felipe Monteiro de Carvalho wrote:
> Hi, the patch looks good, but I would like that some things be changed:
just apply it, it's already changed this morning, so I can continue.
>
> * DoTheJob is the typical name that a function shouldn't have (i.e. it
> doesn't mean anything).
yes, I know leave it there at this moment, I'll remove it as soon as I finish
SetScrollInfo.
>
> * Also, I think that internal functions should be avoided whenever
> possible, and particularly in this case I see that it could be
> avoided. at the point where it is called we only have:
>
> + DoTheJob(SB_HORZ);
> + Result := FScrollInfo.nPos;
>
> for all cases (only in 1 we don't have, but then we could call Exit on
> it) this is the last thing done, so it could be moved out of the case
> statement. We can also use a variable to see later if SB_HORZ or
> something else was selected.
you're right but this is an alpha code at the moment , beautification comes at
the end :)
>
> The only problem is that the function would become too big. If another
> function is really needed I would rather have a private function on
> TQtWidgetSet class. This will force the function to not use the
> private variables of the main one, and thus improve it's design.
>
> * Identation:
>
> case VariableX of
> CONST_A:
> begin
> end;
>
> CONST_B:
> begin
> end;
> end;
>
> each identation step takes 2 spaces and the begin is on separate line
> as the constant
yes , mom ;)
I'll send new patch this afternoon (Fixed some problems with ScrollBars
showing, problem was in calling BoundsRect instead of ClientRect ....)
More information about the Qt
mailing list