Best practice for coding

[dinodod]

I have my little project in which i have TONS of Case statements in it. WHAT i WOULD LIEK TO KNOW S what might be the cleanest method for optimizing my code? I am making heavy use of Functions and trying to keep the functions with the tab / gui section they belong to.

I had 1 file with ALL the functions in it but that became a nighmare to go thru due to the number of lines of code in it.

Thoughts?

[Uten]

Depends on how time critical your code is.

Maybee something like this?

Func ArgCracker($arg)
  Switch $arg
    Case 1 to 20
        Args1to20($arg)
    Case 21 to 40
    Case 41 to 60
  EndSwitch
EndFunc
Func Args1to20($arg)
  Switch $arg
    Case 1
    Case 2
    Case else
  EndSwitch
EndFunc

[PsaltyDS]

You don't give enough info. Some example code would help a lot...

Is each Case different only some small detail? Maybe an array lookup instead? Or StringRegExp() to test multiple conditions with a single Case? There are lots of ways to go, and no info to base a choice on.

[dinodod]

Sorry for the confusion but I was asking about general coding. I feel like I am making spagetti code rather than radble code. Just was looking for example code to review and see how oters are placing their code

[Valuater]

How about this

expandcollapse popup

; Script Format Demonstration.

; 1. Place required include files.
; 2. Set Autoit options, as needed.
; 3. Declare Variables.
; 4. Set Desired Hot Keys.
; 5. Create the GUI.
; 6. Display the GUI.
; 7. Set/Update control information.
; 8. Set the Tray Menu
; 9. Start the Loop, and "Listen" for a message.
; 10. Create the Functions.


; here is an Example


; 1. Includes
#include <GuiConstants.au3>
#include <String.au3>

; 2. Autoit Options
Opt("GUICloseOnESC", 1)         
;1 = ESC  closes script, 0 = ESC won't close.
opt("TrayMenuMode", 1)   
; Default tray menu items (Script Paused/Exit) will not be shown.
opt("TrayOnEventMode", 1)
;TraySetClick (16); right click

; 3. Declare Variables use, Dim, Global or Local.
Dim $Text, $Cloudy = 1
; see help for more info.

; 4. Set Hot Keys
HotKeySet("{F1}", "About_GUI")
; when F1 is pressed, goto function "About_GUI".

; 5. Create the GUI
$Window = GUICreate("My Weather Program")
$combo = GUICtrlCreateCombo("", 120, 50, 150, 20)
$button = GUICtrlCreateButton("Press here for Daily Weather Report", 100, 300, 200, 40)

; 6. Display the GUI
GUISetState()

; 7. Set the control information

; this will split the string called $days, by each comma.
$days = StringSplit("Sunday,Monday,Tuesday,Wednesday,Thursday,Friday,Saturday", ",")
; the return is an array $days[0] is 7, the total
; $days[1] contains "Sunday" ... $days[7] contains "Saturday"

; this will use a for/next loop to set the $combo information with the $days.
For $x = 1 to $days[0]
    GUICtrlSetData( $combo, $days[$x])
Next

; 8. Set the tray menu
    $About_tray = TrayCreateItem ("About Weather")
    TrayItemSetOnEvent (-1, "About_GUI")
    TrayCreateItem ("")
    $exit_tray = TrayCreateItem ("Exit Weather")
    TrayItemSetOnEvent (-1, "Set_Exit")
    
    TraySetState ()

; 9. Start the loop
While 1
    ; "Listen" for the message
    $message = GUIGetMsg()
    
    If $message = $GUI_EVENT_CLOSE Then Set_Exit()
    
    If $message = $button Then
        ; read the selected day in the combo
        $Text = GUICtrlRead( $combo )
        ; call the function with the info
        Day_Function($Text)
    EndIf

WEnd

; 10. Create the Functions 

Func Day_Function($Text)
    If $Text = "" Then Return
    ; randomly choose amount of rain
    $rain = Random(5, 100, 1)
    ; call another function for clouds
    Cloud_Function()
    If $Cloudy = 1 Then
        $Clouds = "Cloudy"
    Else
        $Clouds = "Clear"
    EndIf
    ; show the weather information.
    MsgBox(64, "Your Weather","On " & $Text & " You can expect,   " & @CRLF & $Clouds & " skies in the afternoon and evening   " & @CRLF & _
    "The chance of rain is approximately " & $rain & " percent   ")
EndFunc

Func Cloud_Function()
    If $Cloudy = 1 Then
        $Cloudy = 2
        ; leave this function now
        Return
    EndIf
    If $Cloudy = 2 Then
        $Cloudy = 1
        Return
    EndIf
EndFunc

Func About_GUI()
    MsgBox(64, "Welcome!", " this is your Weather Service      ")
EndFunc    

Func Set_Exit()
    MsgBox(0,"","Good-Bye!",1)
    Exit
EndFunc
    
; All of these functions are UDF's, User Defined Functions

; When you have completed a Script use Tidy under "Tools" above to "clean" it up.
; Then make a comment at the top, like this

#cs ===============================================================================
    *AutoIt 1-2-3     ver 1.0.1 - 02.01.2006
    Autor:             Valuater
    E-mail:            XPCleanMenu@aol.com
    Language:       English
    OSystem:         Windows Xp
    Requirements:     Legal copy of Microsoft Windows Xp
    Construction:     AutoIt 3.1.1+, SciTE 1.64
    Features:         Automated Program, Reads Text, Runs Scripts, etc
    - Use = Tutorials, Help Files, Installers, Presentations, etc
    - ...
    
    Thanks to all, Enjoy...
#ce ===============================================================================

8)

[ChrisL]

Sorry for the confusion but I was asking about general coding. I feel like I am making spagetti code rather than radble code. Just was looking for example code to review and see how oters are placing their code

Sometimes your better writing it down in plain english which for me seems to organize it better in my head.

Sometimes I write a load of crap and then go back and clean it up later.

Sometimes I just write crap but if it aint broke I don't fix it!

[AutoItKing]

Put the functions into separate files and just use #include.

[birdofprey]

Put the functions into separate files and just use #include.

I get trouble with undeclared variables if I do that. How can I over come that ? (I'm not going to check the list of variables defined at the beginning one by one in order to move some to a separate way, if that's the only way I'm going to stick to my old method.)

I use #region, #endregion to define regions for functions that fit in a certain category. I just close that category if it's in my way.

If you use Scite, you might make good use of Ctrl-F2 and F2 as well if you tend to go lost on the page.

Another thing that could help you, is... "functionizing" ? It helps you "put complicated things into one line", the line on which you call the function. (No offense, you were talking about spaghetti... )

Anyhow, regions help me. Hope they help you too.

Any other suggestions ?

[Edit] Changed identation to make reply more readable

Edited May 1, 2007 by birdofprey

[Uten]

I get trouble with undeclared variables if I do that. How can I over come that ? (I'm not going to check the list of variables defined at the beginning one by one in order to move some to a separate way, if that's the only way I'm going to stick to my old method.)

I use #region, #endregion to define regions for functions that fit in a certain category. I just close that category if it's in my way.

If you use Scite, you might make good use of Ctrl-F2 and F2 as well if you tend to go lost on the page.

Another thing that could help you, is... "functionizing" ? It helps you "put complicated things into one line", the line on which you call the function. (No offense, you were talking about spaghetti... )

Anyhow, regions help me. Hope they help you too.

Any other suggestions ?

[Edit] Changed identation to make reply more readable

Always declare variables.

Try to eliminate the bad habit basic languages teaches you and wrap everything into functions. Only globals, includes, one exit and one function call outside a function declaration. And yes Always start function names and assosiated global variables (if needed) with a descriptive name. Much like all functions are named in AutoIt. Win*, GuiCtrl*, Process* and so one.

#include <mylib.au3>
Global $gDbg

Func MyFunc1()
    Return 4
EndFunc

Func AppMain()
    ;Do waht you usually do here.
EndFunc
;If in trouble always place this call as the last one in the script.
; Then you should be reasonably sure all functions are loaded.
AppMain()
Exit

[birdofprey]

Always declare variables.

Try to eliminate the bad habit basic languages teaches you and wrap everything into functions. Only globals, includes, one exit and one function call outside a function declaration. And yes Always start function names and assosiated global variables (if needed) with a descriptive name. Much like all functions are named in AutoIt. Win*, GuiCtrl*, Process* and so one.

I'm not sure if that's enough. Of course, descriptive names are an absolute must. And wrapping everything into functions... that's what I meant with "functionzing".

I'm not going to check the list of variables defined at the beginning one by one in order to move some to a separate file, if that's the only way I'm going to stick to my old method.

I mistakenly used the wrong a word in my previous post. It was very late in the night when I posted that, sorry. And you haven't addressed the issue well because of that probably. And why is declaring variables that important ? I'd honestly prefer not to, and really'd like if global variables could be declared anywhere ( Maybe it's just Scite that complaines...)

[Uten]

And I would like to walk straight across the road withot looking. Quite often I can, but you know.... Sometimes I get in troble when I do..

[birdofprey]

And I would like to walk straight across the road withot looking. Quite often I can, but you know.... Sometimes I get in troble when I do..

I see your point. Maybe I should just use even more descriptive names for the variables, that might help with selecting them in case I later decide that I want to move parts of my script to a different file, which I'll include afterwards.

I'm a self-made programmer only. Maybe one of those of you who know a little more could tell me what the logic or idea behind positioning the declaration of the variables at the top is...

Edit: typo... it's late again...

Edited May 2, 2007 by birdofprey

[jefhal]

I find that, now that I've got a 100 plus scripts written, I have now started going back and re-writing the most commonly used ones and cleaning up all my bad code (tons). I'm turning all the frequently used code into functions, as suggested above, and putting them into includes. Common names can be used for computer name $strComputer; for msgbox repsonses $resp; for active directory variables $ADvariable, etc. You might also want to use an addon to Scite that captures and stores code snippets, so that you don't have to re-type every little chunk of common code that you write (or "borrow"). Finally, looking at some of the master coder's code on this forum will teach you a lot too!

[NELyon]

IMO, if your making a closed source project, It's not as important to have bad coding practices as long as the code you have works (It is somewhat important though) but Open Source is different.

I'm not saying you should neglect good coding practices though

[Uten]

IMO, if your making a closed source project, It's not as important to have bad coding practices as long as the code you have works (It is somewhat important though) but Open Source is different.

I'm not saying you should neglect good coding practices though

Or you could say that it seldom does unless you have a recognizable style in your writing. Even worse go back and understand what you have done a few weeks from now. That is hard enough with structured code..

[dinodod]

In regards to using Regions, that is something I don't see much use for other than grouping code. I use SCITE and I love to be able to collapse code into sections but it's not customizable as far as what you want to allow to be collapsable. I.E. Regions are collapsable and so are Functions. I want to prevent SCITE from allowing Functions to be collapsable so I can use Regions more.

I use the heck out of Functions so keep my main code cleaner and simple to follow. I hate the fact you should declare all your variables (Unless it is needed). Never understood that. All variables should be a default type of String (Since in most cases, that is what they end up being) and if you need a variable to be of a different type, then DECLARE it (Global or of another type, etc)

Thanks for you comments on this...