Looking for advice

[Q2D]

So, my script does not work 100% for what I want, but I'll get it there eventually. What I'm looking for now is help making my code look nicer. I'm pretty new to coding in general, but I think "looking good" is necessary. What I've been doing is pecking away at this, writing piece by piece, adding, taking away, and copy/paste'ing. It's now a mix of parts of several different scripts I've been playing with in an effort to be lazy at work. What I've wound up with is (in my opinion) not good. It gets the job done (for the most part) but I really want it to look nicer. If this was your code snippet, how would you structure it differently? Clean it up? How would you have written it so it looks nicer, so it looks like it's efficient, professional code? I'm not looking for someone to rewrite this for me, I'm looking to learn how to write prettier, more efficient scripts. If you happen to respond with changes, I'd like to know why you chose to make those particular changes. Thanks!!

expandcollapse popup

#include <ButtonConstants.au3>
#include <EditConstants.au3>
#include <GUIConstantsEx.au3>
#include <WindowsConstants.au3>
#include <IE.au3>
_IEErrorHandlerRegister()

Opt("GUIResizeMode", $GUI_DOCKRIGHT)
$Form1_1 = GUICreate("IDM Search", 341, 195, 192, 124)
$MenuItem1 = GUICtrlCreateMenu("&File")
$mnuOpen = GUICtrlCreateMenuItem("Open", $MenuItem1)
$mnuSave = GUICtrlCreateMenuItem("Save", $MenuItem1)
$nmuExit = GUICtrlCreateMenuItem("Exit", $MenuItem1)
$MenuItem2 = GUICtrlCreateMenu("&Help")
$mnuHelp = GUICtrlCreateMenuItem("&Help", $MenuItem2)
$MenuItem3 = GUICtrlCreateMenu("&About")
$Google = GUICtrlCreateInput("Google", 8, 112, 121, 21)
GUICtrlSetTip(-1, "Google")
$Idm = GUICtrlCreateButton("Go!", 136, 112, 35, 25)
GUISetState(@SW_SHOW)

Func MyExit()
    Exit
EndFunc

While 1
    $nMsg = GUIGetMsg()
    Switch $nMsg
        Case $GUI_EVENT_CLOSE
            Exit
        Case $nmuExit()
            Exit
        Case $mnuOpen
            Local $sFileToOpen = ""
                $sFileToOpen = FileOpenDialog ("Select a file", "C:\", "Text Files(*.txt)")
        Case $mnuSave
            MsgBox("", "Save?", "Save what? There's nothing to save!?!")
        Case $mnuHelp
            ShellExecute("http://www.autoitscript.com/wiki/Main_Page")
        Case $Idm
            $oIE = _IECreate()
_IENavigate($oIE, "https://nowayjose.com")

$o_form = _IEFormGetObjByName($oIE, "aspnetForm")
Func _WinWaitActivate($title,$text,$timeout=0)
    WinWait($title,$text,$timeout)
    If Not WinActive($title,$text) Then WinActivate($title,$text)
    WinWaitActive($title,$text,$timeout)
    EndFunc
$o_login = _IEFormElementGetObjByName($o_form, "ctl00$ContentPlaceHolder1$Login1$UserName")
$o_password = _IEFormElementGetObjByName($o_form, "ctl00$ContentPlaceHolder1$Login1$Password")

_IEFormElementSetValue($o_login, "me")
_IEFormElementSetValue($o_password, "anpassword")

Send("{TAB 9}")
Send("{ENTER}")

Func _WinWaitActivate2($title,$text,$timeout=0)
    WinWait($title,$text,$timeout)
    If Not WinActive($title,$text) Then WinActivate($title,$text)
    WinWaitActive($title,$text,$timeout)
EndFunc

$o_form = _IEFormGetObjByName($oIE, "aspnetForm")

$o_login = _IEFormElementGetObjByName($o_form, "ctl00_ContentPlaceHolder1_TextBox1")

_IEFormElementSetValue($o_login, "das*")
    EndSwitch
WEnd

The ultimate goal for this is to eventually have something that takes input, logs me into a web form, and submits that input to the web form. It doesn't quite work yet, but that's not my concern at the moment. I'll finish this puzzle after I learn how to clean all of the pieces.

[BrewManNH]

First I'd move every single function out of the While loop. It doesn't belong there and just makes your code very hard to follow. Actually, they can't be located there they will cause the script to crash, not sure what I was thinking before.

Close every one of your functions with an EndFunc, because right now you have several that aren't closed and I can't tell where one starts and another ends.

Edited August 10, 2011 by BrewManNH

[wakillon]

I can't try script

I get an error for https://nowayjose.com : Internet Explorer cannot display the webpage

expandcollapse popup

#include <ButtonConstants.au3>
#include <EditConstants.au3>
#include <GUIConstantsEx.au3>
#include <WindowsConstants.au3>
#include <IE.au3>
_IEErrorHandlerRegister()

Opt("GUIResizeMode", $GUI_DOCKRIGHT)
$Form1_1 = GUICreate("IDM Search", 341, 195, 192, 124)
$MenuItem1 = GUICtrlCreateMenu("&File")
$mnuOpen = GUICtrlCreateMenuItem("Open", $MenuItem1)
$mnuSave = GUICtrlCreateMenuItem("Save", $MenuItem1)
$nmuExit = GUICtrlCreateMenuItem("Exit", $MenuItem1)
$MenuItem2 = GUICtrlCreateMenu("&Help")
$mnuHelp = GUICtrlCreateMenuItem("&Help", $MenuItem2)
$MenuItem3 = GUICtrlCreateMenu("&About")
$Google = GUICtrlCreateInput("Google", 8, 112, 121, 21)
GUICtrlSetTip(-1, "Google")
$Idm = GUICtrlCreateButton("Go!", 136, 112, 35, 25)
GUISetState(@SW_SHOW)

While 1
    $nMsg = GUIGetMsg()
    Switch $nMsg
        Case $GUI_EVENT_CLOSE
            Exit
        Case $nmuExit()
            Exit
        Case $mnuOpen
            Local $sFileToOpen = ""
                $sFileToOpen = FileOpenDialog ("Select a file", "C:\", "Text Files(*.txt)")
        Case $mnuSave
            MsgBox("", "Save?", "Save what? There's nothing to save!?!")
        Case $mnuHelp
            ShellExecute("http://www.autoitscript.com/wiki/Main_Page")
        Case $Idm
            $oIE = _IECreate("https://nowayjose.com")
            $o_form = _IEFormGetObjByName($oIE, "aspnetForm")
            $o_login = _IEFormElementGetObjByName($o_form, "ctl00$ContentPlaceHolder1$Login1$UserName")
            $o_password = _IEFormElementGetObjByName($o_form, "ctl00$ContentPlaceHolder1$Login1$Password")
            _IEFormElementSetValue($o_login, "me")
            _IEFormElementSetValue($o_password, "anpassword")
            Send("{TAB 9}")
            Send("{ENTER}")
            $o_form = _IEFormGetObjByName($oIE, "aspnetForm")
            $o_login = _IEFormElementGetObjByName($o_form, "ctl00_ContentPlaceHolder1_TextBox1")
            _IEFormElementSetValue($o_login, "das*")
    EndSwitch
WEnd


Func _WinWaitActivate($title,$text,$timeout=0)
    WinWait($title,$text,$timeout)
    If Not WinActive($title,$text) Then WinActivate($title,$text)
    WinWaitActive($title,$text,$timeout)
EndFunc

Func _WinWaitActivate2($title,$text,$timeout=0)
    WinWait($title,$text,$timeout)
    If Not WinActive($title,$text) Then WinActivate($title,$text)
    WinWaitActive($title,$text,$timeout)
EndFunc

Func MyExit()
    Exit
EndFunc

[Q2D]

First I'd move every single function out of the While loop. It doesn't belong there and just makes your code very hard to follow. Actually, they can't be located there they will cause the script to crash, not sure what I was thinking before.

Close every one of your functions with an EndFunc, because right now you have several that aren't closed and I can't tell where one starts and another ends.

I do get a few syntax errors in the debug output (ERROR: missing Wend, REF: missing Wend, ERROR: syntax error), but the script does kind-of function. Thank you for giving me somewhere to start.

I can't try script

I get an error for https://nowayjose.com : Internet Explorer cannot display the webpage

Sorry, that's definitely my fault. That's not the real url of the page I'm working with. I didn't want to leave the real web page in there. It's an internal website that we use for account validation, resets, and monitoring logins. However, I can tell you that this does log me in just fine once it gets there.

[edena]

An advice from a programmer I heard some time ago saying if you're new to programming or writing scripts you have to write down the process, that the software will do for you.

For example: You want a GUI(Graphic User Interface) with a button that when pressed will open the AutoIt website...

So

1. You have the GUI script here which has a button and

2. You have the script to create a web browser

#include <ButtonConstants.au3>

#include <GUIConstantsEx.au3>

#include <WindowsConstants.au3>

#include <IE.au3>

#Region ### START Koda GUI section ### Form=

$Form1 = GUICreate("Form1", 200, 64, 192, 114)

$Button1 = GUICtrlCreateButton("Button1", 48, 16, 91, 33)

GUISetState(@SW_SHOW)

#EndRegion ### END Koda GUI section ###

3. and a script that opens AutoIt when the button is pressed.

While 1

$nMsg = GUIGetMsg()

Switch $nMsg

Case $GUI_EVENT_CLOSE

Exit

Case $Button1

$oIE = _IECreate ("www.autoitscript.com")

EndSwitch

WEnd

Hope this helps.