[Qt] TScrollBar patch
Felipe Monteiro de Carvalho
felipemonteiro.carvalho at gmail.com
Mon May 28 23:19:36 CEST 2007
Hi, the patch looks good, but I would like that some things be changed:
* DoTheJob is the typical name that a function shouldn't have (i.e. it
doesn't mean anything).
* 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.
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
Carefully written code means it will be easier to maintain later =)
thanks,
--
Felipe Monteiro de Carvalho
More information about the Qt
mailing list