Dalija Prasnikar

1 hour ago, Anders Melander said:

I haven't QP'd it and will probably not have time to do so anytime soon; We've just updated our main product suite to Delphi 11.2 (from 10.3) and there's plenty of other stuff that needs my attention.

I just looked at that code and the problem is not in the inline variable. Casting as interface creates hidden interface reference no matter what and that reference hinders release. This behavior is not a regression and it is not related to inline variables so I wouldn't hope it will be fixed soon.

Reported as https://quality.embarcadero.com/browse/RSP-40166 

5 hours ago, Anders Melander said:

Casting a function result with the interface as operator and the old behavior is back. Reproduced with Delphi 11.2.

I suppose you tested those outside the main code block. Have you filed QP report?

11 minutes ago, microtronx said:

Since we're all crazy programmers ... is "No" acceptable? Any explanation of why No?

It is complicated...

In theory you can have Windows application that has multiple GUI threads with each one holding own windows message loop.

The answer is no, because VCL is build on the premise of single main (GUI) thread (like many other frameworks as there is really no advantage of having such multithreaded GUI) and you would have to write your own framework to achieve such application and probably would have to rewrite significant parts of the RTL.

No.

25 minutes ago, John R. said:

Nice! I'm currently using events from Spring4D and at first sight, this looks similar. Or am I missing something ?

I have no idea how similar it is to Spring Events as I never used them. 

9 hours ago, juergen said:

So I would also be very happy to see a simple example.

Here is simple example:

Declare event in some independent unit C

type
  TMyEvent = type string;

Unit A

procedure TFormA.ButtonClick(Sender: TObject);
begin
  NxHorizon.Instance.Post<TMyEvent>('New Caption');
end;

 
Unit B

type
  TFormB = class(...
    procedure FormCreate(Sender: TObject);
    procedure FormDestroy(Sender: TObject);
  protected 
    fMyEventSubcription: INxEventSubscription;
    procedure OnMyEvent(const aEvent: TMyEvent);    
  end;
    
procedure TFormB.FormCreate(Sender: TObject);
begin
  fMyEventSubcription: := NxHorizon.Instance.Subscribe<TMyEvent>(MainAsync, OnMyEvent);
end;

procedure TFormB.FormDestroy(Sender: TObject);
begin
  if Assigned(fMyEventSubscription) then NxHorizon.Instance.Unsubscribe(fMyEventSubscription);
end;

procedure TFormB.OnMyEvent(const aEvent: TMyEvent);
begin
  Label.Caption := aEvent;
end;

Since you are working with GUI, events should be dispatched either as MainAsync or MainSync and that will ensure they run in the context of the main thread. Also, because they run on the main thread, you can just unsubscribe and you don't need to WaitFor subscription to finish work as there will be no active work done in the background threads.

I have put subscribing and unsubscribing in FormCreate and FormDestroy event handlers, but you can also put them anywhere else where you are doing form initialization or finalization. Or, you can subscribe and unsubscribe at some other point.

6 hours ago, Jeff Steinkamp said:

I took a page out of this Indy demo and applied it to my code, and once I got the FileStream creating straightened out for the thread, it is functioning quite well indeed!.  I have attached the file for that unit if you want to have a look and see if there is anything that can be done differently.

It just needs few tweaks.

First, you don't need Application.ProcessMessages anywhere - they have been commented out, but the whole point of threads is getting rid of those. So you can just forget about it 🙂

Next, this will work fine, but in case user closes the dialog before download completes already queued synchronization methods will access released form and crash. To prevent that, you need to add additional filed in Form declaration and OnCloseQuery event handler on the form. I also fixed try finally block inside thread as constructing HTTP instance can also fail (not likely event, but still), and in that case the cleanup was not proper one. I also added try..except block to show error message in case of failure. 

If the whole application is closed then anonymous threads can be killed at any time. If this is a possibility, then you might want to replace TThread.CreateAnyonymousThread with TTask.Run (without Start) from System.Threading as all running tasks will be waited for on application shutdown. 

FileDownload.pas

FileDownload.dfm

You can find simple demo how to use threads with Indy here: https://github.com/dalijap/code-delphi-thread-safety/tree/master/Part3/19 Indy

This demo is using anonymous thread, but you can also use regular threads or TTask from Parallel Programming library, too. 

If you can post your code I can give you more detailed advice.

1 hour ago, Fr0sT.Brutal said:

Probably he meant that in case of many parameters they are pushed to stack not to registers. Stack is somewhat related to memory 🙂

Uh.... this is such far fetched speed optimization that it is not even worth thinking about.

And if you start measuring particular solutions, you may easily find that you will not get very far with them.

1 hour ago, Stano said:

I am a layman. I remember that the number of arguments 3 is related to memory management.

Memory management has nothing to do with number of parameters in constructors.

There are other reasons why having too many parameters is considered "bad", but those are all in line that having too many parameters can be indication that your class is doing too much - violates Single Responsibility Principle.

In real life, having classes and functions that require a lot of parameters and that are not in violation of SRP, nor can be adequately split, are quite common, and often workarounds for solving such "issues" are more convoluted than having few parameters over the top.

If there is really a need to have class fully initialized during construction and too many parameters poses an issue that cannot be meaningfully solved by splitting the class, then Builder pattern can be suitable solution. There is usually a lot of repetitive code involved in writing a Builder class, so it is useful only if you need to construct particular class in plenty of places across your application.

The main problem with many parameters, especially when they are of same type, is that you can easily pass the wrong data to wrong parameter. This could be solved on IDE level, by showing names of the parameters as hints within code - not visible outside the IDE,, so calling the constructor or function would look like 

TPerson.Create(AFirstName: 'John', ALastName: 'Doe');

Another similar feature on language level are named parameters, where you can or must explicitly write parameter name in the code, similar to the above.

5 minutes ago, Ian Branch said:

Where does that go and why?

Implementation section where your TTracksAddEditForm class is declared. Just like with any other method that is declared in a class, its implementation must be included inside that unit implementation section. 

21 minutes ago, Ian Branch said:

.  Using a constructor.  Never knowingly done so.  I tried Dalija's constructor but it gives me an E@065 Unsatisfied forward or external declaration: 'TTracksAddEditForm.Create'.  Not an issue, I don't believe I will go that route.

Oops... I forgot to copy-paste that part of the code... 

constructor TTracksAddEditForm.Create(AOwner: TComponent; const AsMode: string; AiAlbum: integer; const AsTrack: string);
begin
  inherited Create(AOwner);
  FsMode := AsMode;
  FiAlbum := AiAlbum;
  FsTrack := AsTrack;
end;

1 hour ago, Ian Branch said:

I guess I should ask, is there a better way to do this?

Your code can be simplified in several ways. I will write each step separately, so you can more easily adapt your code if you need additional functionality beyond the code shown here.

First, as I mentioned in previous post, you don't need to write getter and setter functions at all, if you just read or write field in those and noting more. First optimization would be removing all that redundant code.  This is not just speed optimization, but also makes cleaner code that is easier to read and follow. This code is functionally completely equivalent to your declaration and you would create and initialize the form the same way you do now.

  TTracksAddEditForm = class(TForm)
  private
    FsMode: string;
    FiAlbum: integer;
    FsTrack: string;
  public
    property sMode: string read FsMode write FsMode;
    property iAlbum: integer read FiAlbum write FiAlbum;
    property sTrack: string read FsTrack write FsTrack;
  end;

Your next question was whether you need to have both getters and setters. If you are just setting property, then you don't need getter. Similarly, if you have read only property that you will just read, then you don't need setter.

So next optimization would be following. There is no difference in how you create and initialize your form.

  TTracksAddEditForm = class(TForm)
  private
    FsMode: string;
    FiAlbum: integer;
    FsTrack: string;
  public
    property sMode: string write FsMode;
    property iAlbum: integer write FiAlbum;
    property sTrack: string write FsTrack;
  end;

procedure TAudioForm.btnInsertTrackClick(Sender: TObject);
begin
  TracksAddEditForm := TTracksAddEditForm.Create(Self);
  TracksAddEditForm.sMode := 'Insert';
  TracksAddEditForm.iAlbum := Albums.FieldByName('Index').AsInteger;
  TracksAddEditForm.sTrack := TracksView.FieldByName('Title').AsString;
  TracksAddEditForm.Show;
end;

But, the best way to initialize any class that you will construct through code (without bringing in dependency injection) would be declaring all required information as parameters to the constructor, Such code would also require different code when constructing form. 

  TTracksAddEditForm = class(TForm)
  private
    FsMode: string;
    FiAlbum: integer;
    FsTrack: string;
  public
    constructor Create(AOwner: TComponent; const AsMode: string; AiAlbum: integer; const AsTrack: string); reintroduce;
  end;

procedure TAudioForm.btnInsertTrackClick(Sender: TObject);
begin
  TracksAddEditForm := TTracksAddEditForm.Create(Self, 'Insert', Albums.FieldByName('Index').AsInteger, TracksView.FieldByName('Title').AsString);
  TracksAddEditForm.Show;
end;

The advantage of passing all required data as parameters during construction process is that you cannot accidentally forget to initialize some required field. If some fields are optional, then you can stick to initializing through properties, but using simplified examples before this last one.

59 minutes ago, Ian Branch said:

Can some one point me to a plain English explanation of what Getters & Setters do and how they do it.

Getters and Setters are methods called when you get (read) or set (write) property. In Delphi you don't need to have getter and setter methods. Properties can be directly connected to a field. 

You use getters or setters when you want to run additional code besides directly reading or writing some field value. 

Following is basic property declaration without any getters or setters. When you access property, compiler just directly reads or writes from associated field.

  TFoo = class
  protected
    FValue: Integer;
  published
    property Value: Integer read FValue write FValue;
  end;

Following is property declaration with setter method (you can have any combination you need and you don't need to declare both methods if you just need one of them)

  TFoo = class
  protected
    FValue: Integer;
    procedure SetValue(AValue: Integer);
  published
    property Value: Integer read FValue write SetValue;
  end;

Because getters and setters are methods, they will be a bit slower than directly using a field. If your setter is just setting a field and does nothing more, then you don't really need it. Same goes for getter. If it just returns the field then it is not needed.

procedure TFoo.SetValue(AValue: Integer);
begin
  FValue := AValue;
end;

If for instance you don't want to allow setting Value to zero, you might have following code in a setter.

procedure TFoo.SetValue(AValue: Integer);
begin
  if AValue <> 0 then
    FValue := AValue;
end;

You can even raise an exception if you set property to invalid value or similar.

The only time when you would need to use getters and setters that just read or write to a field, is when your property needs to be accessible through interface. Interfaces require that getters and setters are methods. 

14 minutes ago, Uwe Raabe said:

Under Tools - Configure Tools add a new entry named Kill LSP with the following settings:

 

Code:

Program: taskkill
Parameters: /IM DelphiLSP.exe /F

Can you please post that as Q/A on Stack Overflow. It will be more easily found there.

Loading resource strings should be thread-safe. Issue probably happens because you are loading string resource to a shared string variable. Without seeing your code it is impossible to tell exactly.

11 hours ago, Ian Branch said:

Leaving aside the use of 'FreeAndNil()' and 'with' discussions, to me this is a simpler structure, easier to code and read, and it works.  Those things make my life easier.. 😉

I don't think you can avoid that 

'with' has many pitfalls and replacing code that does not use 'with' with code using 'with' is not something I would recommend doing. 

11 hours ago, Ian Branch said:

So To my question - As you can see in the first code I use FreeAndNil(), as much for peace of mind, imagined or real, as anything else.

Is there a way to implement the equivalent in the second code structure?

You have to ask yourself what is the purpose of FreeAndNil? In the case of local variable the reason (regardless whether we consider it valid or not) is to prevent accidental access of the object instance after it has been released. In such cases we hope that accessing nil reference would crash immediately revealing the obvious bug in the code.

However, when you use 'with' you no longer have explicit reference to the object that you can accidentally access outside the 'with' block. Yes, if you are really sloppy, you can still call Free first and that call some other code if call to Free is not the last thing you do in the 'with' block. Main point is compiler hides the reference, there is nothing you can nil here, and compiler will certainly not access the instance outside the 'with' block by mistake.

Calling Destroy in destructor without having nil check first is definitely incorrect code that can lead to memory leaks as destructor needs to be able to handle partially constructed instances. That is why Free is commonly used as it does not require sprinkling destructors with nil checks before calling Destroy. 

But, very likely this bad code in the destructor is not the primary cause of the issues you are seeing and that there is more going on. If the destructor is at fault, then this means that the instance was not properly constructed and that construction raised an exception. Otherwise the core problem is in some other code we don't see here.

20 hours ago, DavidOne said:

What do you think about this solution?

Is it really thread safe and may be a good workaround?

This is appropriate solution.

Code that runs inside those anonymous methods will run in the context of the main thread (this is defined by first boolean parameter passed to ExecuteAsync - if you pass False there then that code will run in the context of the background thread). 

Because it runs in the context of the main thread it will be thread-safe the same way the same way your original function is thread-safe if you run it from the main thread.

The only thing you need to pay attention to, is that RESTRequest, RESTClient, and RESTResponse used for making asynchronous request can only handle single request at the time in thread-safe manner. That means, you cannot run UserRESTRequest.ExecuteAsync again while the previous request is not yet completed. Once the request completes, you can reuse those REST components to make another request. 

There are two separate concerns here.

First, main application code block can exhibit some weird behavior for global variables declared there, including inline variables. If you want to make proper test of some behavior, I suggest wrapping the code in additional procedure. 

Second, inline variables had (and maybe still have) some problems and sometimes using them does not work as expected. If you encounter an issue, usually solution is to declare local variable instead of inline.

Your text case does not work correctly in 10.3.3 when code runs in main code block. If moved to the procedure, then it behaves as it should. In 10.4.2 and and 11.2 it works correctly in all scenarios, so the issue was fixed in the meantime.

They route license increase to Sales and Renewals, and of course, Sales will try to sell you a subscription, but AFAIK you don't have to purchase it and they will bump your license count. Still, this is confusing practice and sometimes you need to be persistent. 

What you want to do is impossible in Delphi (without bringing bad coding practice like Application.ProcessMessages in the process)

You cannot have function that will return the result of some asynchronous operation. Period.

Asynchronous REST request has events that will run when request is successfully or unsuccessfully finished. In those events you can add logic that needs to run as the result of the operation. 

procedure TMainForm.ButtonClick(Sender: TObject);
begin
  RESTRequest.ExecuteAsync(
    procedure
    begin
      Memo.Lines.Add(RESTResponse.Content);
    end,
    True, True,
    procedure(Error: TObject)
    begin
      Memo.Lines.Add(Exception(Error).Message);
    end);
end;

Another way, by running request in task or another thread uses similar approach. In such case you would transform your function into a procedure and pass callback (procedure, method or anonymous method) that will run within thread after the request is completed. If you need to run that code in the context of the main thread you can use TThread.Synchronize or TThread.Queue

7 hours ago, Epo said:

Thank you, Now it works perfectly!

Bug report is now available to public:

Cannot expand columns or rows while expand style is in fixed size

https://quality.embarcadero.com/browse/RSP-39228

55 minutes ago, Epo said:

Hi,

I have Installed without problems 11.2 using the migration tool after (except for GetIt package). But each time the IDE start, before the welcome screen is displayed, an error is raised:

Cannot add columns or rows while expand style is fixed size. If the button OK is pushed, the IDE is launched correctly.

That is known (reported issue). There is some problem with migrating or applying Welcome screen layout after migration.

When you launch IDE click Edit Layout on Welcome Screen. Reset Welcome Screen to default layout and then adjust it again the way you like it.

Next time you start IDE it should run normally.

6 hours ago, Vincent Parrett said:

If that were the case, then you wouln't need to change anything at all in SynEdit - but you do as it cannot see SetAdditionalPCREOptions.

There are some scenarios where which helper will be visible does not work as expected.

Last helper in scope is not always visible https://quality.embarcadero.com/browse/RSP-38448