Leaderboard

20% discount on delphihtmlcomponents.com products. Please use coupon code NY2022 (valid until end of the year). This is the last chance to order HTML Library at the current price. Prices will be increased in January, but renewal price (including a 15% discount increased by 5% every year) will remain the same for customers with continued subscription. https://delphihtmlcomponents.com/order.html

I have plenty of similar constructions in my DB app to edit some tables. So init is fully contained in form c-tor/design props/onShow, changes are applied internally and result is not important.

When you're displaying the forms this way they are useless there are no input values to the form and the form doesn't produce any result. You're destroying the instance before the calling code could read the results back. I am aware that you can initialize the form in the formcreate and write the results back in formdestroy but that is a bad design.

You aren't going to be able to display the contents of a binary file in a rich edit control like this. If you want a hex editor, then display each byte as hex.

I see several issues with that code: 1. your SetLength() is useless, because the subsequent ReadBytes() is completely replacing your allocated array with a different array allocates by ReadBytes(). 2. your SetString() is wrong, for two reasons: a. String is UnicodeString in D2009+, which you are clearly using due to TEncoding, which was also introduced in D2009. You are trying to populate the UnicodeString with AnsiChar data, but there is no overload of SetString() for that. You can only populate a UnicodeString with WideChar data. For AnsiChar data, you need to use AnsiString instead. b. You are advancing a PAnsiChar pointer through the byte array, but you are giving the array's whole length to SetString(), which means SetString() is going to read beyond the end of the array into surrounding memory after the 1st loop iteration. Since you are trying to convert each Byte into an (Ansi)String, you would need to use 1 for the SetString() length instead. 3. appending strings to the RichEdit1.Text property in a loop is VERY inefficient. It requires reading the RichEdit's entire content into a String in memory, then expanding that String in memory with appended data, then replacing the RichEdit's entire content with the new string. A much more efficient way to append strings to a RichEdit's content is to use its SelText property instead, eg: RichEdit1.SelStart := RichEdit1.GetTextLen; RichEdit1.SelLength := 0; RichEdit1.SelText := ...; But even that is not the best approach in this situation. Since you are trying to display a byte array as-is in the RichEdit as text, you may as well just assign the RichEdit.Text property 1 time with the entire array as 1 string, no loop needed at all, eg: var AFile: TFileStream; BR: TBinaryReader; MyByteArray: TBytes; mstr: AnsiString; begin AFile := TFileStream.Create(filename, fmOpenRead); BR := TBinaryReader.Create(AFile, TEncoding.Unicode, false); try // Get the whole file contents into the byte array MyByteArray := BR.ReadBytes(AFile.Size); SetString(mstr, PAnsiChar(@MyByteArray[0]), Length(MyByteArray)); RichEdit1.Text := mstr; // or: RichEdit1.Text := TEncoding.ANSI.GetString(MyByteArray); BR.Close; finally BR.Free; AFile.Free; end; end; In which case, you may as well just get rid of the TBinaryReader, as you can read bytes from the TFileStream directly: var AFile: TFileStream; MyByteArray: TBytes; mstr: AnsiString; begin AFile := TFileStream.Create(filename, fmOpenRead); try SetLength(MyByteArray, AFile.Size); // Get the whole file contents into the byte array AFile.ReadBuffer(MyByteArray[0], SetLength(MyByteArray)); SetString(mstr, PAnsiChar(@MyByteArray[0]), Length(MyByteArray)); RichEdit1.Text := mstr; // or: RichEdit1.Text := TEncoding.ANSI.GetString(MyByteArray); finally AFile.Free; end; end; In which case, you can just get rid of the TFileStream altogether, too: RichEdit1.Text := TFile.ReadAllText(filename, TEncoding.ANSI); Or RichEdit1.Lines.LoadFromFile(filename, TEncoding.ANSI);

There is a function for that in System.IOUtils: MyByteArray := TFile.ReadAllBytes(filename);

It will work fine so long as you use the virtual constructor and all your classes override that correctly. If you passed in an anon method that creates and returns a newly minted form, that would allow other constructors to be used. To me this is pointless though. It's a really common pattern, it's only a handful of lines. I don't really see the benefit. I mean, if you were trying to centralise some logging or other aspect, then it would make sense. Otherwise I don't really see the benefit. Also the name sucks. Should include the fact that the form is shown modally.

Nearly all forms I show modal return a result. At the most basic level all these forms are inherited from one base form which has this code: class function TBaseDialogForm.ShowDialog(AOwner: TComponent): Boolean; begin with Create(AOwner) do try Result := ShowModal = mrOK; finally Free; end; end; class function ShowDialog(AOwner: TComponent): Boolean; virtual;

well, in this case you could derive TMyForm from TForm which has a .Show[Modal]: returntype; method and do many more things for every form in one place, and no need for a global "utils.pas". Just edit the class declaration at the top of the file from class(TForm) to class(TMyForm) (after you implemented TMyForm of course)