|
Post by Mark Dunham on Apr 23, 2018 11:19:43 GMT -5
I am refactoring a bunch of code and I am wondering which is better. A button that uses a branch label to execute it's code or a subroutine. The code snippet shows the two methods of implementing the same thing but I am wondering which was is better. - Is one better than the other?
- Is there a performance gain?
- Are there any other things to take into consideration?
bmpbutton #Main.btn1, "Button1.bmp", [LavaBrick], UL,10,10 bmpbutton #Main.btn2, "Button1.bmp", LavaBrick, UL,10,50
open "Scroll Static Text" for graphics as #Main #Main, "trapclose [Main.Close]"
wait [LavaBrick] 'SOME REALLY AWESOME CODE GOES HERE wait
[Main.Close] close #Main end wait
'SUBROUTINE sub LavaBrick lavabrick$ 'SOME REALLY AWESOME CODE GOES HERE end sub
|
|
|
Post by Alyce Watson on Apr 23, 2018 11:26:06 GMT -5
I believe a branch label might execute faster in some cases. Brent posted about this years ago. Code inside branch labels is visible everywhere in the main program. On the other hand, a branch label indicates an implied "goto" and some people firmly believe that gotos should never be used. Code placed in subs is visible only within the sub. I'd suggest picking one style of handler and sticking with it.
|
|
|
Post by tsh73 on Apr 23, 2018 12:05:47 GMT -5
You can use same event handler SUB for several buttons - checking inside what HANDLE$ you got. You cannot pull the trick with branch label.
As for speed - then pushing buttons human always be slower link, so don't bother.
Have a look:
'Form created with the help of Freeform 3 v07-31-2015 'Generated on Apr 23, 2018 at 19:57:49
list$="Eeny Meeny Miny Moe" r=int(rnd(0)*4)+1 '1..4 global secret$ secret$ = word$(list$, r)
nomainwin
button #main.Eeny,"Eeny",guessWho, UL, 10, 27, 96, 25 button #main.Meeny,"Meeny",guessWho, UL, 10, 62, 96, 25 button #main.Miny,"Miny",guessWho, UL, 10, 97, 96, 25 button #main.Moe,"Moe",guessWho, UL, 10, 132, 96, 25
open "Guess the Who" for window as #main print #main, "font ms_sans_serif 10" print #main, "trapclose [quit.main]"
[main.inputLoop] 'wait here for input event wait
sub guessWho handle$ who$=word$(handle$,2,".") if who$=secret$ then notice "Congrats! You've found it!"+chr$(13)+"It was ";who$ close #main end else notice "Not here yet! You've have to look more!"+chr$(13)+"It was't ";who$ #handle$ "!disable" end if end sub
[quit.main] 'End the program close #main end
|
|
|
Post by Mark Dunham on Apr 23, 2018 12:09:47 GMT -5
I believe a branch label might execute faster in some cases. Brent posted about this years ago. Code inside branch labels is visible everywhere in the main program. On the other hand, a branch label indicates an implied "goto" and some people firmly believe that gotos should never be used. Code placed in subs is visible only within the sub. I'd suggest picking one style of handler and sticking with it. Thanks Alyce. I have learned OOP and I am familiar with the statement "GOTO'S" are bad lol. I always try to pick a standard and be consistent thank you as always.
|
|
|
Post by Mark Dunham on Apr 23, 2018 12:14:47 GMT -5
You can use same event handler SUB for several buttons - checking inside what HANDLE$ you got. You cannot pull the trick with branch label. As for speed - then pushing buttons human always be slower link, so don't bother. Have a look: 'Form created with the help of Freeform 3 v07-31-2015 'Generated on Apr 23, 2018 at 19:57:49
list$="Eeny Meeny Miny Moe" r=int(rnd(0)*4)+1 '1..4 global secret$ secret$ = word$(list$, r)
nomainwin
button #main.Eeny,"Eeny",guessWho, UL, 10, 27, 96, 25 button #main.Meeny,"Meeny",guessWho, UL, 10, 62, 96, 25 button #main.Miny,"Miny",guessWho, UL, 10, 97, 96, 25 button #main.Moe,"Moe",guessWho, UL, 10, 132, 96, 25
open "Guess the Who" for window as #main print #main, "font ms_sans_serif 10" print #main, "trapclose [quit.main]"
[main.inputLoop] 'wait here for input event wait
sub guessWho handle$ who$=word$(handle$,2,".") if who$=secret$ then notice "Congrats! You've found it!"+chr$(13)+"It was ";who$ close #main end else notice "Not here yet! You've have to look more!"+chr$(13)+"It was't ";who$ #handle$ "!disable" end if end sub
[quit.main] 'End the program close #main end
That gives me more food for thought on how I should refactor the section of code I am about to work on. At this time it is bmpbuttons that let you select the tile you want to draw to the screen it selects the tile/sprite and closes the window. If I could do that with one subroutine vs. 20 depending on how many tiles are on that screen it would save repetitive code. Thanks for the example.
|
|
cundo
Full Member
Muchas Gracias!!
Posts: 146
|
Post by cundo on Apr 23, 2018 14:06:49 GMT -5
I tell you, I really like Branch labels, when you see those brackets in your code, you know you are using Liberty BASIC. I mostly start my codes using branch labels and GOTO, then at some point, I "improve" my code, removing the GOTOs, and placing some SUBS and FUNCTIONS instead of branches. Of course, sometimes I go with SUB routines directly from the start, whatever it comes to my mind I start typing.
|
|
|
Post by Mark Dunham on Apr 23, 2018 14:25:41 GMT -5
I tell you, I really like Branch labels, when you see those brackets in your code, you know you are using Liberty BASIC. I mostly start my codes using branch labels and GOTO, then at some point, I "improve" my code, removing the GOTOs, and placing some SUBS and FUNCTIONS instead of branches. Of course, sometimes I go with SUB routines directly from the start, whatever it comes to my mind I start typing. That is how the program I am refactoring came to be I was learning and I had an idea and now almost 2 years later I am like I bet I can make it better. And I can with all that I have learned. Love to hear what others think. I spent a few years within a software group and the personalities can sometimes be challenging. The LB forum has always been open and friendly. Many thanks.
|
|
|
Post by laboyd2 on Apr 23, 2018 16:26:04 GMT -5
My own personal preference, I have not used GOTO for many years. It is a holdover from the early days when spaghetti code was common.
|
|
|
Post by Mark Dunham on Apr 23, 2018 19:14:01 GMT -5
My own personal preference, I have not used GOTO for many years. It is a holdover from the early days when spaghetti code was common. I am trying to avoid spaghetti code.
|
|
|
Post by Mark Dunham on Apr 23, 2018 21:49:43 GMT -5
Thank you for all the advice in this thread I took what was advised and this is the subroutine that I came up with I listed the short version of the subroutine but it is now 1 subroutine and not 25 of them. I will add more of my buttons to the case statement which will simplify my code. LB forums rocks. Thank you all.
sub WallEvent handle$
buttHandle$=word$(handle$,2,".")
select case buttHandle$ case "RedBrickbtn" tileSelected$ = "RedBrick" previousTile$ = "RedBrick" #LE.GB, "addsprite redBrick brickR" #LE.GB, "spritexy redBrick -100 -100" end select close #Walls end sub
|
|
|
Post by tsh73 on Apr 24, 2018 0:31:07 GMT -5
Mark, having single sub with huge SELECT CASE inside will not differ much from having 25 branch sections. How about use consistent naming to produce all string constant from handle name? (well, at least most of them)
|
|
|
Post by Mark Dunham on Apr 24, 2018 7:03:03 GMT -5
Mark, having single sub with huge SELECT CASE inside will not differ much from having 25 branch sections. How about use consistent naming to produce all string constant from handle name? (well, at least most of them) Do you have an example that I could look at for clarity. When I look at my code I see multiple ways of accomplishing what I am trying to do. All the methods work but my goal is to have less code that is more efficient I am loving the input it has me really analyzing my code. I have a few very large case statements in the refactor by doing that I reduced my code so far by 1000 lines I hope to reduce another 1000 by the time I am done maybe more with the input everyone is providing
|
|
|
Post by tsh73 on Apr 27, 2018 3:13:16 GMT -5
Hello Mark sorry for long-no-answer
See code you posted:
sub WallEvent handle$
buttHandle$=word$(handle$,2,".")
select case buttHandle$ case "RedBrickbtn" tileSelected$ = "RedBrick" previousTile$ = "RedBrick" #LE.GB, "addsprite redBrick brickR" #LE.GB, "spritexy redBrick -100 -100" end select close #Walls end sub it has * button handle RedBrickbtn * tileSelected RedBrick * sprite name redBrick * bitmap name brickR Problem is the other buttons have more varuations (capital first button etc)
Now suppose your button handle still "RedBrickbtn" but all other names will be just "RedBrick"
Then you sub could be written as
sub WallEvent2 handle$ buttHandle$=word$(handle$,2,".") '"RedBrickbtn" name$ = left$( buttHandle$, len( buttHandle$)-3) '"RedBrick" tileSelected$ = name$ previousTile$ = name$ #LE.GB, "addsprite ";name$;" ";name$ #LE.GB, "spritexy ";name$;" -100 -100" close #Walls end sub
and it would work for all buttons!
Well, may be having bunch of different object named "RedBrick" is not very good idea. You can uses prefix/suffix on that, like sprRedBrick, bmpRedBrick or RedBrickSpr, RedBrickBMP (I prefer first because it's easier to get part past 3 letter (mid$(buttHandle$, 4)) then part without last three letters (left$( buttHandle$, len( buttHandle$)-3)) - but you may as well never need that at all!) -- the point is, all these names should be calculate-able from handle.
PS I hope you've made tileSelected$ , previousTile$ global?
PS2 in the ZIP file you've posted on subject I found lot's of single-dimension arrays, like 4 for each sprite. I really think using 2-dimention array(s) with one index is internal sprite number (say 1 for RedBrick, 2 for soil etc) will help reducing code - now you will be able to write block working with arrays for every sprite, having sprite number as parameter.
|
|
|
Post by Mark Dunham on Apr 27, 2018 6:46:22 GMT -5
Hello Mark sorry for long-no-answer See code you posted: sub WallEvent handle$
buttHandle$=word$(handle$,2,".")
select case buttHandle$ case "RedBrickbtn" tileSelected$ = "RedBrick" previousTile$ = "RedBrick" #LE.GB, "addsprite redBrick brickR" #LE.GB, "spritexy redBrick -100 -100" end select close #Walls end sub it has * button handle RedBrickbtn * tileSelected RedBrick * sprite name redBrick * bitmap name brickR Problem is the other buttons have more varuations (capital first button etc) Now suppose your button handle still "RedBrickbtn" but all other names will be just "RedBrick" Then you sub could be written as sub WallEvent2 handle$ buttHandle$=word$(handle$,2,".") '"RedBrickbtn" name$ = left$( buttHandle$, len( buttHandle$)-3) '"RedBrick" tileSelected$ = name$ previousTile$ = name$ #LE.GB, "addsprite ";name$;" ";name$ #LE.GB, "spritexy ";name$;" -100 -100" close #Walls end sub
and it would work for all buttons! Well, may be having bunch of different object named "RedBrick" is not very good idea. You can uses prefix/suffix on that, like sprRedBrick, bmpRedBrick or RedBrickSpr, RedBrickBMP (I prefer first because it's easier to get part past 3 letter (mid$(buttHandle$, 4)) then part without last three letters (left$( buttHandle$, len( buttHandle$)-3)) - but you may as well never need that at all!) -- the point is, all these names should be calculate-able from handle.
PS I hope you've made tileSelected$ , previousTile$ global? PS2 in the ZIP file you've posted on subject I found lot's of single-dimension arrays, like 4 for each sprite. I really think using 2-dimention array(s) with one index is internal sprite number (say 1 for RedBrick, 2 for soil etc) will help reducing code - now you will be able to write block working with arrays for every sprite, having sprite number as parameter. tsh73 no worries thanks for the response and the example. I am going to play around with that as it will reduce the amount of code I have even more. In the refactoring I have done I now only have one array I was able to use just one which makes my code cleaner. I have reduced the amount of code almost by half. I have made tileSelected$ and previousTile$ global it was part of my refactor. When I get the refactor completed I will post the code in a new thread and list all that was changed. I like what the program does I just want to optimize the code. Thanks for all the feedback.
|
|