[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