Jump to content

Recommended Posts

I am using delphi fmx for windows 10. I have listbox that will have list of messages. every message is inserted in that list box as a fmx frame. I decided to use this as a frame because I can get more nicer GUI look.

I am simulating creating and deleting messages ( Frames ) by clicking on buttons. So I am basically making front end of the application and I want to make it good working before i actually hook it to internet.

 

I am trying to practice oop and good design patterns so that is why i want to share with you how i did all this so you can give me suggestions how to improve.

 

So I have listbox, when i click button1 i am triggering this code that will add 10 messages ( 10 frames ) to a listbox.

for i := 0 to 10 do
  begin
 randNumb := Random(9900);
    messageList[randNumb] := TListBoxItem.Create(Self);
    messageList[randNumb].Parent := ListBox1;
    messageList[randNumb].Selectable := False;
    TFrame1.Create(Self).Name := IntToStr(randNumb);
    with TFrame1(FindComponent(IntToStr(randNumb))) do
    begin
      Parent := messageList[randNumb];
      //Frame ID is public int property in Frame unit, i will need it later when I am deleting message ( frame )
      frameId := randNumb;
      Label1.Text := 'Message body';
      LabelName.Text := 'John Doe';
      Country.Images := ImageList1;
    end;

MessageList represents public property ->  MessageList : Array [0 .. 9900] of TListBoxItem; I set array size from 0 to 9900... randNumb presents random number ( index ) that will be given to every messsage ( frame )

Property frameId have same value as name of the frame that i am creating dynamically.

 

When I want to delete message ( frame ) I first click on the frame. When I click on the frame I am triggering OnClick event that is located in Frame unit, after that I am deleting message ( frame ) by using "DisposeOf". I have noticed that i also have to call "DisposeOf" not just on frame, i have to call it on MessageList also. So i am also doing MessageList[frameId].DisposeOf;   and frame is deleted , also item in MessageList is deleted.

 

**********************

Everything so far that I wrote is working but it does not feel right... 

1. I want to add new message ( frame ) with an ID that is incremented by 1 compared to biggest index in my array. SO if i have 10 messages , when I add next message I want that message index to be 10+1 =11..

2.Lets say that i have 10 messages , and i delete message with index of 5. What will happen with other message indexes ? is my MessageList array going to shrink or it will just leave empty space where is index 5 ? Will message that had index 6 before deleting message 5 go and drop down for one place and have index 5 ? will message with index 7 drop on 6th place and so on... ?

3.I could possibly make MessageList array with unknown length and everytime when i want to add new message i can check length of MessageList array and resize it for +1 and add that item on the end.. but still i have feeling that this is not a way to make this ...

 

I was reading this : https://stackoverflow.com/questions/433221/how-do-i-declare-an-array-when-i-dont-know-the-length-until-run-time

 

Third message says  " If your array is no longer static, you might want to consider using a TList, a TStringList or one of the many container classes in the Contnrs unit.

They may better represent what you are doing, provide additional capabilities you might need, e.g. sorting or name/value pairs, they dynamically grow as you need them, and have been very well optimized. "

 

So my conclusion about this is that I have to find some correlation between TList or TStringList that i have to create and  MessageList : Array [0 .. 9900] of TListBoxItem that I already created because my list of messages ( Frames ) will grow and shrink dynamically and I dont want to manually figure out how to do it.. I am not sure, maybe my whole concept is wrong from beginning 

 

Thank you for your help

 

 

Edited by Tntman

Share this post


Link to post

This doesn't seem to have anything to do with either OOP or the subject of Design Patterns. And your description is too microscopic to give the reader (this one, anyway) a grasp of what you're trying to accomplish. Like if you talked about feeding a string through several holes, using different criss-cross patterns, and making a knot at one end ... not many people would conclude you're talking about putting a shoelace on a shoe and tying it.

 

Your description focuses more on the string, the holes, and the criss-cross pattern than what you're doing with them. Does that make sense?

 

It's like you bought 1000 toilet seats and you're trying to figure out what to do with them by describing the kinds of toilets they fit on, when what you're really interested in is something like building public toilets in a part of the world where they don't have many of them. In other words, you're not focused on the right thing.

 

Generally speaking, arrays are good for storing things if you have a finite number of items that can be indexed and accessed by their positions relative to each other. They're good for representing physical associations. Reserved parking spaces in a parking area, desks in a classroom, or seats in a restaurant. You'd typically number them 1..n and then assign each number to a person for some period of time. There's a one-to-one relationship, so you could refer to each one by their assigned number, or their name. At some point, the relationship expires, and the spot can be reallocated to someone else. The spot itself does not disappear, just the relationship it has to another person or object. That is, you don't "destroy" a parking spot, a desk in a classroom, or a table in a restaurant, right? In practice, you clean them up and re-use them. Similarly, you don't destroy a slot in an array, you simply clear it out and re-use it.

 

If you have an inventory of something that is refreshed periodically, and it is consumed at a different rate by others, then you want a list in both sides. There's no relationship between items in one and consumers in the other. (It's called a "consumer-producer" problem.) There may be an ordering in terms of how inventory is acquired and then distributed, like First-In-First-Out (FIFO) or Last-In-First-Out (LIFO) or maybe its random.  There may be a maximum capacity (the size of a warehouse is fixed, but rarely filled to capacity). You could also have the inventory broken down by some attribute like size, shape, color, texture, some kind of date, etc., and stored in places related by those attributes. Things on a list usually come inside of containers, and you typically dispose of the containers once you hand out the goods inside of them. While you have them under your control, however, you use the containers to move the stuff around and identify what they contain.

 

Anyway, I've discussed a number of examples here at a level of detail that I bet is sufficient for you to build a model around each one with no further information, based simply on your familiarity with the subject mentioned.

 

You've used a lot of words to explain something at a level of detail that is nearly impossible for someone to know what you're talking about. That's ok, it's a common problem people have when learning new concepts. I myself tend to be very verbose because I like to be very precise and explicit, so I usually say too much. But it's usually all relevant.

 

Why don't you try setting aside what you've written so far, and start over beginning with a one-sentence description of the problem you're trying to solve? If you make it similar to something that most people would understand, you'll need far fewer words to explain it. And you'll have a much more clear understanding of it in your own mind as well.

 

BTW, this is called a "use-case model". 

Edited by David Schwartz
  • Like 1

Share this post


Link to post

TListBox ist not ideal for large datasets, consider TListView instead

  • Like 1

Share this post


Link to post

Thank you for your answers i enjoyed reading them 🙂

 

Looks like that I made problem without problem. I overcomplicated everything for unknown reason ... IN short, my app receive messages and user can delete them. Every message have parent listbox and every message is basically custom looking frame that i made. So simply when use receive new message I am creating new frame that will have Listbox as a parent and frame name will be message_id. That way i will not have errors because every frame will have unique name ( message_id ). When user deletes message I am just simply looping frames and if that particular frame exists i am removing it. 

Share this post


Link to post
On 5/5/2020 at 6:38 PM, Tntman said:

 


for i := 0 to 10 do
  begin
    randNumb := Random(9900);
    messageList[randNumb] := TListBoxItem.Create(Self);

 

Based on your luck, you can leak 10 TListBoxItem obects with one run. Don't do this. Just because it did not fail during debugging, it might fail in real life.

Random(9900) will give you a number between 0 and 9899, leaving the last one (#9900) unused 100% of the times. Consider using Random(High(messageList) + 1) instead of a burned-in number; so if you decide to resize your array later on - you only have to do it at one place.

On 5/5/2020 at 6:38 PM, Tntman said:

 


    TFrame1.Create(Self).Name := IntToStr(randNumb);
    with TFrame1(FindComponent(IntToStr(randNumb))) do

 

If you really like With that much, you can just say

With TFrame1.Create(Self) Do
 Begin
  Name := IntToStr(randNumb);
  [...]
 End;

Burning CPU cycles for FindComponent is completely useless, if you just created that object. I'd use a local variable here, but that's only my taste.

 

As for the rest of your fears - they are all valid. I guess @David Heffernan has an alert set up if you write SetLength(myArray, Length(myArray) + 1) anywhere, as he will appear and tell you that it is a REALLY bad practice. Indexing will also never automatically shrink, items will not be moved - you have to do it manually if you want to...

 

... oooor instead of an array you can say

messageList: TObjectList<TListBoxItem>;
[...]
messageList := TObjectList<TListBoxItem>.Create(True);

...this way you will get an automatically expanding and shrinking list, which will automatically .Free your object if you delete it from the list.

With this, the above With block could be replaced by...

Var
 index: Integer;
Begin
 index := messageList.Add(TListBoxItem.Create(Self));
 messageList[a].Name := xxx;
 [...]

which looks cleaner imo.

 

P.s.: Is there a purpose you are naming dynamically created objects? I never ever had to do it so far; thus can not imagine a reason why I'd need to do so.

Edited by aehimself
  • Thanks 1

Share this post


Link to post

Your answer was really helpful, i almost finished my example today but i can see that i will edit it tomorrow based on your post.. 

 

5 hours ago, aehimself said:

Based on your luck, you can leak 10 TListBoxItem obects with one run. Don't do this.

This is something that i did just for demo to see what will happen if i load 'n' amount of messages ( frames) 'at the same time', basically just for testing purposes to see how fast it will perform and to simulate message received.. 

 

5 hours ago, aehimself said:

If you really like With that much, you can just say


With TFrame1.Create(Self) Do
 Begin
  Name := IntToStr(randNumb);
  [...]
 End;

I did not know that I could do this, it really saves time and boost performance knowing that i will have 2k messages for example ( looping over 2k frames) i will edit that part in my code based on your advice.. 

 

5 hours ago, aehimself said:

SetLength(myArray, Length(myArray) + 1)

In my first post i mentioned this that it looks bad way of doing things so i just abandoned this idea

5 hours ago, aehimself said:

oooor instead of an array you can say


messageList: TObjectList<TListBoxItem>;
[...]
messageList := TObjectList<TListBoxItem>.Create(True);

This looks interesting but as I said i think i dont need array of anything in this case because when i add Tframe ( message ) component dynamically on listbox i will give a new tframe component name of message ID, so basically i will never get error that component with particular name already exists because ID's are all different.. . When removing message im just grabbing component name ( tframe name) and performing a look ( looping to find that frame in listbox) and after that i am performing disposeOf. 

5 hours ago, aehimself said:

 

P.s.: Is there a purpose you are naming dynamically created objects?

Well my gui that i made is really intensive and i decided to use frame to represent a message in chat room, so when new message arrives i am creating new tframe ( message) and attaching it to parent listbox, if i dont name it or if i name 2 components with same name im getting error so that is why i am giving unique name to every tframe component

Edited by Tntman

Share this post


Link to post
Procedure TAEMultiSQLMainForm.ConnectionClick(inSender: TObject);
Var
 ts: TTabSheet;
 sqf: TSQLConnectionFrame;
 s: String;
Begin
 s := (inSender As TMenuItem).Caption;

 ts := TTabSheet.Create(Display);
 ts.Caption := s;
 ts.ImageIndex := 1;
 ts.PageControl := Display;

 sqf := TSQLConnectionFrame.Create(ts, Settings.Connections[s], Log);
 sqf.Parent := ts;

 RecolorSQLFrame(ts);
 Display.ActivePage := ts;
 ChangeFocus;
 sqf.ReconnectButton.Click;
End;

This is the code snipplet I use in a tabbed SQL browser to open a new connection; which is also a frame. I am creating a tabsheet and a frame without a name and never got an error concerning component names. That's the only reason I asked.

Share this post


Link to post

My application is connecting on twitch irc and reading msgs from there.. Its not from DB so it is more dynamic, admins on twitch irc can delete some message or users can delete so i have to update particular frame if message is deleted... Thats why i am using frames with name so i can figure out what to delete or edit... I think we are not talking about same frame types, i am using this type of frames :  

 

Edited by Tntman

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×