Recent

Author Topic: Class operator "as" - misused?  (Read 5860 times)

engkin

  • Hero Member
  • *****
  • Posts: 3112
Class operator "as" - misused?
« on: September 21, 2014, 06:12:51 am »
This is my first topic, it started here when Taazz pointed out:
Quote
to tell you the truth I'm having hard time finding any use of "as" it makes no sense to me what so ever so when I see it used it always rings some bells on how it is used and how useful it might be for my programming style, thats why I'm aware of it almost always.

After doing a case-sensitive search for " as T" in the source code of the compiler and its RTL I came with the impression that class operator as is rarely used there. Repeating the same search within the IDE/LCL revealed more usage. Moving randomly between these results gave me the feeling that the class operator is being misused.

It basically gets translated into a call to fpc_do_as:
Code: [Select]
    function fpc_do_as(aclass : tclass;aobject : tobject): tobject;[public,alias: 'FPC_DO_AS']; compilerproc;
      begin
         if assigned(aobject) and not(aobject.inheritsfrom(aclass)) then
           handleerroraddrframeInd(219,get_pc_addr,get_frame);
         result := aobject;
      end;

Which calls:
Code: [Select]
      class function TObject.InheritsFrom(aclass : TClass) : Boolean;

        var
           vmt: PVmt;

        begin
           if assigned(aclass) then
             begin
               vmt:=PVmt(self);
               while assigned(vmt) and (vmt <> PVmt(aclass)) do
                 vmt := vmt^.vParent;
               InheritsFrom := (vmt = PVmt(aclass));
             end
           else
             inheritsFrom := False;
        end;

I don't think it is possible to get the compiler to treat this class operator as a simple typecast.

Am I wrong to believe that it was mainly misused in the source code?

Thank you.

---------------------------------------
A few samples:

customdrawn_common.pas:
Code: [Select]
  if ADest is TCanvas then
  begin
    (ADest as TCanvas).Pixels[0, ASize.cy-1] := WIN2000_FRAME_WHITE;
    (ADest as TCanvas).Pixels[ASize.cx-1, FCaptionMiddle] := WIN2000_FRAME_WHITE;
  end;

  // ToDo: Make the caption smaller if it is too big
  lCaption := AStateEx.Caption;
  if ADest is TCanvas then lTextSize := (ADest as TCanvas).TextExtent(lCaption)
  else lTextSize := Size(50, AStateEx.Font.Size);

StdActns.pas:
Code: [Select]
{ TEditSelectAll }

procedure TEditSelectAll.ExecuteTarget(Target: TObject);
begin
  (Target as TCustomEdit).SelectAll;
end;

procedure TEditSelectAll.UpdateTarget(Target: TObject);
begin
  Enabled := (Target as TCustomEdit).Text <> '';
end;

{ TEditUndo }

procedure TEditUndo.ExecuteTarget(Target: TObject);
begin
  (Target as TCustomEdit).Undo;
end;

procedure TEditUndo.UpdateTarget(Target: TObject);
begin
  Enabled := (Target as TCustomEdit).CanUndo;
end;

menuitem.inc
Code: [Select]
procedure TMenuItem.SetChildOrder(Child: TComponent; Order: Integer);
begin
  (Child as TMenuItem).MenuIndex := Order;

lcl\interfaces\carbon\carbonbars.pp
Code: [Select]
procedure TCarbonProgressBar.CreateWidget(const AParams: TCreateParams);
var
  ProgressBar: TCustomProgressBar;
  Control: ControlRef;
begin
  ProgressBar := LCLObject as TCustomProgressBar;

\lcl\interfaces\gtk\gtkpagecallback.inc
Code: [Select]
function PageIconWidgetExposeAfter(Widget: PGtkWidget; Event: PGDKEventExpose;
  Data: gPointer): GBoolean; cdecl;
...
  ThePage := TObject(Data) as TCustomPage;

jarto

  • Full Member
  • ***
  • Posts: 106
Re: Class operator "as" - misused?
« Reply #1 on: September 21, 2014, 10:27:42 am »
Great find engkin. At least that TCanvas-example is misuse.

When you do: (Sender as TWhatever).YaddaYadda, the compiler checks that Sender really is TWhatever. You get a nice error msg if you make a mistake.

When you do TWhatever(Sender).YaddaYadda, there are no checks and if you call it with wrong kind of objects, random bad stuff happens :-)

Obviously, if you're 100% sure that the class is right, don't use "as".

BeniBela

  • Hero Member
  • *****
  • Posts: 905
    • homepage
Re: Class operator "as" - misused?
« Reply #2 on: September 21, 2014, 10:43:42 am »
That calls for a compiler change: If there was a previous (X as T) or true (X is T) on the current code path, replace all (X as T) with T(X)

engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: Class operator "as" - misused?
« Reply #3 on: September 21, 2014, 04:56:41 pm »
Great find engkin. At least that TCanvas-example is misuse.
Thank you.  :) It was Taazz who pointed out the problem with "as" usage. The TCanvas example is a random one, there are more. What about the less obvious ones?

When you do: (Sender as TWhatever).YaddaYadda, the compiler checks that Sender really is TWhatever. You get a nice error msg if you make a mistake.
Yes, that's true, the compiler throws an exception (run-time error 219) "Invalid type cast". If you have debug information, you can find out the line that caused it as well.

When you do TWhatever(Sender).YaddaYadda, there are no checks and if you call it with wrong kind of objects, random bad stuff happens :-)
Correct, errors here are going to be hard to catch.

Obviously, if you're 100% sure that the class is right, don't use "as".
If you are *not* 100% sure that the class is right, aren't you going to check the class first with "is"? or do you really prefer to receive an exception?

In the few samples presented above, do you think that "as" usage was needed in every case?

engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: Class operator "as" - misused?
« Reply #4 on: September 21, 2014, 04:59:10 pm »
That calls for a compiler change: If there was a previous (X as T) or true (X is T) on the current code path, replace all (X as T) with T(X)
Eventually, I believe this should happen and it should work with the obvious cases. What about the other cases?

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 11383
  • FPC developer.
Re: Class operator "as" - misused?
« Reply #5 on: September 22, 2014, 10:16:55 am »
I rarely use AS.

I usually use a cast, and a assert (x IS TSomeClass);

So for testing, one enables assertions, and then run without any such overhead.

engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: Class operator "as" - misused?
« Reply #6 on: September 22, 2014, 06:54:56 pm »
I rarely use AS.

I usually use a cast, and a assert (x IS TSomeClass);

So for testing, one enables assertions, and then run without any such overhead.
This sounds like a perfect alternative for AS. Do you use an assert every time you use a cast?

----
One more sample where AS was used more like a cast five times:
\components\synedit\syncompletion.pas
Code: [Select]
procedure TSynCompletion.ProcessSynCommand...
...
  i := IndexOfEditor(Sender as TCustomSynEdit);
  if i >= 0 then begin
    with sender as TCustomSynEdit do begin
      if not ReadOnly then begin
        Editor := Sender as TCustomSynEdit; // Will set Form.SetCurrentEditor
        Execute(GetPreviousToken(Sender as TCustomSynEdit), Sender as TCustomSynEdit);
...

taazz

  • Hero Member
  • *****
  • Posts: 5368
Re: Class operator "as" - misused?
« Reply #7 on: September 22, 2014, 06:59:15 pm »
I rarely use AS.

I usually use a cast, and a assert (x IS TSomeClass);

So for testing, one enables assertions, and then run without any such overhead.

Well if you are ok when something goes wrong to see an exception of the worst kind and have your application come crashing down is ok then just use "as" less to type and at least has a milder reaction to errors.

The point of "is" is to be used a guardian at run time it doesn't matter debug or release.
Good judgement is the result of experience … Experience is the result of bad judgement.

OS : Windows 7 64 bit
Laz: Lazarus 1.4.4 FPC 2.6.4 i386-win32-win32/win64

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 11383
  • FPC developer.
Re: Class operator "as" - misused?
« Reply #8 on: September 22, 2014, 07:12:08 pm »
I rarely use AS.

I usually use a cast, and a assert (x IS TSomeClass);

So for testing, one enables assertions, and then run without any such overhead.
This sounds like a perfect alternative for AS. Do you use an assert every time you use a cast?

No, only in certain more complicated scenarios.

Note that my work is in Delphi, so I have to improvise; FPC has this built in with -CR.

 

engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: Class operator "as" - misused?
« Reply #9 on: September 23, 2014, 12:54:37 am »
@Taazz, there is no doubt about "is". The problem that "as" is being misused as a type cast, not as a guard against error.

----
One more example:

tachartaxis.pas
Code: [Select]
constructor TChartAxis.Create(ACollection: TCollection);
begin
  inherited Create(ACollection, ACollection.Owner as TCustomChart);
  FAxisPen := TChartAxisPen.Create;
  FAxisPen.OnChange := @StyleChanged;
  FListener := TListener.Create(@FTransformations, @StyleChanged);
  FMarks := TChartAxisMarks.Create(ACollection.Owner as TCustomChart);
  FMinors := TChartMinorAxisList.Create(Self);
  FPositionUnits := cuPercent;
  FRange := TChartRange.Create(ACollection.Owner as TCustomChart);
  TickLength := DEF_TICK_LENGTH;
  FTitle := TChartAxisTitle.Create(ACollection.Owner as TCustomChart);
  FMarginsForMarks := true;
end;

In this one it was used four times "ACollection.Owner as TCustomChart"

taazz

  • Hero Member
  • *****
  • Posts: 5368
Re: Class operator "as" - misused?
« Reply #10 on: September 23, 2014, 01:03:55 am »
@Taazz, there is no doubt about "is". The problem that "as" is being misused as a type cast, not as a guard against error.

Never said it was, I was just commenting on the need to remove the check from run time after the debugging is over, it makes me shrug a bit.

Quote
----
One more example:

tachartaxis.pas
Code: [Select]
constructor TChartAxis.Create(ACollection: TCollection);
begin
  inherited Create(ACollection, ACollection.Owner as TCustomChart);
  FAxisPen := TChartAxisPen.Create;
  FAxisPen.OnChange := @StyleChanged;
  FListener := TListener.Create(@FTransformations, @StyleChanged);
  FMarks := TChartAxisMarks.Create(ACollection.Owner as TCustomChart);
  FMinors := TChartMinorAxisList.Create(Self);
  FPositionUnits := cuPercent;
  FRange := TChartRange.Create(ACollection.Owner as TCustomChart);
  TickLength := DEF_TICK_LENGTH;
  FTitle := TChartAxisTitle.Create(ACollection.Owner as TCustomChart);
  FMarginsForMarks := true;
end;

In this one it was used four times "ACollection.Owner as TCustomChart"
except the fact that it was used 4 times in a row wasting cpu it is the only legitimate use I have seen so far. Its use is well thought in here.
Good judgement is the result of experience … Experience is the result of bad judgement.

OS : Windows 7 64 bit
Laz: Lazarus 1.4.4 FPC 2.6.4 i386-win32-win32/win64

engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: Class operator "as" - misused?
« Reply #11 on: September 23, 2014, 03:11:34 am »
Never said it was, I was just commenting on the need to remove the check from run time after the debugging is over, it makes me shrug a bit.
Neither the check nor it's exception are desired, not in any good design. When in doubt "is" should be used, and when it returns false a proper action should be taken, instead of leaving the code at that point to unknown consequences.

When a few TButtons share one OnClick event, there is no point of checking if the Sender is of type TButton, because it is a fact by design. It is perfectly ok to cast the Sender to get, for instance, the Tag value.

When we use TCollection and TCollectionItem as in the following code:
Code: [Select]
  TSomeItem = Class(TCollectionItem)
...
  end;

  TSomeCollection = Class(TCollection)
..
  Public
    Function AddSomeItem(Const AFewParams : String, ...) : TSomeItem;
  end;
...

I used to use "as" because it is more readable. I did not use it for checking.
Code: [Select]
Function TSomeCollection AddSomeItem(Const AFewParams : String, ...) : TSomeItem;
begin
  Result:=Add as TSomeItem;
...
end;

I believe a cast should be used instead:
Code: [Select]
Function TSomeCollection AddSomeItem(Const AFewParams : String, ...) : TSomeItem;
begin
  Result:=TSomeItem(Add);
...
end;

except the fact that it was used 4 times in a row wasting cpu it is the only legitimate use I have seen so far. Its use is well thought in here.
Well, the point here is about the four times. I won't argue about the first one. I believe the programmer did use "as" as a type cast in the last three.

 

TinyPortal © 2005-2018