Forum > LCL
[CLOSED] Result assignments removal in lcl/widgetset/wslclclasses.pp
lagprogramming:
lcl/widgetset/wslclclasses.pp has a series of useless "Result := nil;" lines in:
--- Code: Pascal [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---function FindClassNode(const AComponent: TComponentClass): PClassNode;var idx: integer;begin Result := nil; if WSClassesList.Search(AComponent, idx) then Exit(WSClassesList[idx]); Result := FindNodeParent(AComponent.ClassParent);end;No matter what, the nil value will be overwritten later.
--- Code: Pascal [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---function FindParentWSClassNode(const ANode: PClassNode): PClassNode;begin Result := ANode^.Parent; while Result <> nil do begin if Result^.WSClass <> nil then Exit; Result := Result^.Parent; end; Result := nil;end;There are two ways of leaving the "while Result <> nil do" loop:
- exiting with an assigned result at "if Result^.WSClass <> nil then Exit;" in which case the line "Result := nil;" won't be run;
- exiting when Result is nil, situation when the line "Result := nil;" is useless.
--- Code: Pascal [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---function FindCommonAncestor(const AClass1, AClass2: TClass): TClass;begin Result := AClass1; if AClass2.InheritsFrom(Result) then Exit; Result := AClass2; while Result <> nil do begin if AClass1.InheritsFrom(Result) then Exit; Result := Result.ClassParent; end; Result := nil;end;Similar to function FindParentWSClassNode(const ANode: PClassNode): PClassNode;
There are two ways of leaving the "while Result <> nil do" loop:
- exiting with an assigned result at "if AClass1.InheritsFrom(Result) then Exit;" in which case the line "Result := nil;" won't be run;
- exiting when Result is nil, situation when the line "Result := nil;" is useless.
Edit: Removed the patch because it become obsolete.
Thaddy:
Well, less rude than usual:
You are mixing up Break with Exit. Exit really leaves the method/procedure/function and Break exits the loop. So your patch is useless, unless there is a real bug.
Which is not what I conclude. The result value seems always correct.
See:
https://www.freepascal.org/docs-html/rtl/system/break.html
and
https://www.freepascal.org/docs-html/rtl/system/exit.html
FWIW: I know the difference between Break and Exit can be confusing, but take note.
( It can be even more confusing when refactoring this using continue )
wp:
I think the patches are correct. But I would not apply them because they sacrifice readability without having a noticeable advantage.
lagprogramming:
In addition to removing the "Result := nil;" lines, I've changed the two exits inside the while loops with breaks. Also, at function FindClassNode I've removed the exit by adding an else.
--- Code: Pascal [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---function FindClassNode(const AComponent: TComponentClass): PClassNode;var idx: integer;begin if WSClassesList.Search(AComponent, idx) then Result := WSClassesList[idx] else Result := FindNodeParent(AComponent.ClassParent);end;
--- Code: Pascal [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---function FindParentWSClassNode(const ANode: PClassNode): PClassNode;begin Result := ANode^.Parent; while Result <> nil do begin if Result^.WSClass <> nil then Break; Result := Result^.Parent; end;end;
--- Code: Pascal [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---function FindCommonAncestor(const AClass1, AClass2: TClass): TClass;begin Result := AClass1; if AClass2.InheritsFrom(Result) then Exit; Result := AClass2; while Result <> nil do begin if AClass1.InheritsFrom(Result) then Break; Result := Result.ClassParent; end;end;
wp:
I don't see a difference in using "break" or "exit" here.
Navigation
[0] Message Index
[#] Next page