ssubirias3 Posted August 20, 2007 Posted August 20, 2007 Hi guys, I'm so stoked about this being my first au3 script that has no mouseclicks, no send commands, no winwaits... and for me that's HUGE!!! Of course I couldn't have done this without the help of PsaltyDS, big_daddy, Paulie, MHz, Siao and others schooling me on UBound and Arrays. Like PsaltyDS says, "arrays are our friends" and boy are they! Ok so what does this do? At its core its the same program Czardas put together and asked for help on this thread (http://www.autoitscript.com/forum/index.php?showtopic=50987&hl=zero2one). I was going to offer some suggestion back when he/she first posted the message, but this kind of took on a life of its own and what a learning experience this has been for me! Czardas, thanks for the idea and I hope you like my version. Summary: Takes a 3 digit hex number like 2A5 and offers the possible replacements of 3A5, 6A5, AA5, 2B5, 2E5, 2A7 and 2AD. This is different than Czardas' version in that it allows multiple entries and displays them on the screen before writing the results to a csv file. If you run the script again it will first check against the csv file for matches before moving onto the $avMap array. Also at the end it will update the csv file and sort the file. Purpose of this post: 1 - To ask the code gurus to review and rate the script (keep in mind I don't have a developer background) 2 - See if the code can be reduced from its 343 lines. I'm guessing SmOke_N or PsaltyDS can shrink this down to 100 or less lines. 3 - After having my code torn to pieces by the gurus, I hope to learn some better practices. 4 - See if anyone can find any bugs I may have over looked in my extensive testing. Well enough now for the code! Thanks again for those that helped me, I'm embarrassed to say this took me 2 weeks to write <blush>. PsaltyDS... thumbs up??? expandcollapse popup#include <array.au3> #include <GUIConstants.au3> #Include <GuiListView.au3> #Include <GuiStatusBar.au3> Opt('GUICloseOnESC',1) Dim $avMap[16][2] = [["A","BE"],["B","F"],["C","DE"],["D","F"],["E","F"],["F","X"],["0","1248"] _ ,["1","359"],["2","36A"],["3","7B"],["4","56C"],["5","7D"],["6","7E"],["7","F"],["8","9AC"],["9","BD"]] Dim $avInput[1], $avChr[1][3], $avDb[1][13], $avResults[1][13], $avDbNF[1], $avTemp[1] Dim $file = FileOpen("HexDb.csv", 0), $c = 0, $Lctrl, $DbTotal, $NFTotal _GetInputs() _ReadInDb() _ChkDb() _Results() MsgBox(262208,"Exit Script", "The script has added " & $NFTotal & " records " & _ @CRLF & "and sorted the database.") Func _GetInputs() While 1 $input = InputBox("Input", "Valid hexadecimal characters are 0–9 and A–F. Example: A0F" & _ @CRLF&@CRLF&"Enter your 3 digit hexadecimal value.") $errInBx = @error $f = StringUpper($input) Switch $errInBx Case 1 $Ectrl = "0-cancel" _Exit() $errExit = MsgBox(4+32+0+262144,"Exit Script", "Do you want to exit the script?") Switch $errExit Case 7 If $avInput[0] <> "" Then Exitloop Else ContinueLoop EndIf Case 6 Exit EndSwitch Case 0 Switch $input Case "", StringLen($input) <> 3 Or Not StringIsXDigit($input) $errExit = MsgBox(6+16+256+262144,"Error: Invalid Entry", "You did not enter a valid 3 digit hexidecimal value." _ & @CRLF & @CRLF & "Hexadecimal characters are 0–9 and A–F." _ &@CRLF &"Examples: A0F, 2FF, 00E, BE2" & @CRLF & @CRLF) Switch $errExit Case 11 If $avInput[0] = "" Then _Exit() Else $Lctrl = "continue" Exitloop EndIf Case 10 ContinueLoop Case 2 _Exit() EndSwitch Case $input For $d = 0 To UBound($avInput, 1) -1 If $input = $avInput[$d] Then $errExit = MsgBox(6+16+256+262144,"Error: Duplicate Entry", $avInput[$d] & " has already been entered.") Switch $errExit Case 11 $Lctrl = "continue" ExitLoop Case 10 $Lctrl = "retry" ExitLoop Case 2 _Exit() EndSwitch EndIf Next If $Lctrl = "retry" Then $Lctrl = "" ContinueLoop ElseIf $Lctrl = "continue" Then $Lctrl = "" ExitLoop Else If $c = 0 Then _ArrayPush($avInput, $f) Else _ArrayAdd($avInput, $f) EndIf $results = MsgBox(4+32+0+262144, "Accepted: " & $avInput[$c], "Do you have more values to enter?") $c += 1 Switch $results Case 6 ContinueLoop Case 7 ExitLoop EndSwitch EndIf EndSwitch EndSwitch Wend $msg = "The follow " & UBound($avInput, 1) & " values will be processed to " & _ @CRLF & "determine their replacement values." If UBound($avInput, 1) -1 > 6 Then $sInputs = _ArrayToString( $avInput, ", ", 0, 4) $summary = MsgBox(1+64+0+262144, "Review: Input", $msg &@CRLF & @CRLF & $sInputs & ", ... " & $avInput[UBound($avInput, 1) - 1]) ElseIf UBound($avInput, 1) -1 = 0 Then $summary = MsgBox(1+64+0+262144, "Review: Input", $msg &@CRLF & @CRLF & $avInput[0]) Else $sInputs = _ArrayToString( $avInput, ", ", 0, UBound($avInput, 1) - 1) $summary = MsgBox(1+64+0+262144, "Review: Input", $msg &@CRLF & @CRLF & $sInputs) EndIf If $summary = 2 Then _Exit() EndIf EndFunc ;; ==> _GetInputs() Func _ReadInDb() $file = FileOpen("HexDb.csv", 0) If $file == -1 Then _DbHeader() Else $lnumb = 7 While 1 $line = FileReadLine($file, $lnumb) If @error == -1 Then ExitLoop $avTemp = StringSplit($line, ",") For $c = 1 To $avTemp[0] $avDb[$lnumb -7][$c -1] = $avTemp[$c] Next ReDim $avDb[UBound($avDb, 1) + 1][13] $lnumb += 1 Wend If $avDb[0][0] <> "" And $avDb[UBound($avDb, 1) -1][0] == "" Then While $avDb[UBound($avDb, 1) -1][0] == "" ReDim $avDb[UBound($avDb, 1) - 1][13] Wend EndIf EndIf FileClose($file) EndFunc ;; ==> _ReadInDb() Func _ChkDb() Dim $c = 0, $d = 0 For $m = 0 To UBound($avInput, 1) - 1 $Lctrl = "no-match" For $r = 0 To UBound($avDb, 1) - 1 If $avInput[$m] = $avDb[$r][0] Then $Lctrl = "match" ExitLoop EndIf Next If $Lctrl = "match" Then If $d = 0 Then For $i = 0 To UBound($avDb, 2) - 1 $avResults[$d][$i] = $avDb[$r][$i] Next Else ReDim $avResults[UBound($avResults) + 1][13] For $i = 0 To UBound($avDb, 2) - 1 $avResults[$d][$i] = $avDb[$r][$i] Next EndIf $d += 1 Else If $c = 0 Then _ArrayPush($avDbNF, $avInput[$m]) Else _ArrayAdd($avDbNF, $avInput[$m]) EndIf $c += 1 EndIf Next If $avDbNF[0] <> "" Then _ChkMap() Else $Lctrl = "no-change" EndIf EndFunc ;; ==> _ChkDb() Func _ChkMap() For $i = 0 To UBound($avDbNF, 1) - 1 If $i = 0 Then $avChr[$i][0] = StringMid($avDbNF[$i], 1, 1) $avChr[$i][1] = StringMid($avDbNF[$i], 2, 1) $avChr[$i][2] = StringMid($avDbNF[$i], 3, 1) Else ReDim $avChr[UBound($avChr) + 1][3] $avChr[$i][0] = StringMid($avDbNF[$i], 1, 1) $avChr[$i][1] = StringMid($avDbNF[$i], 2, 1) $avChr[$i][2] = StringMid($avDbNF[$i], 3, 1) EndIf Next For $m = 0 To UBound($avDbNF, 1) -1 $e = 0 For $l = 0 To 2 For $r = 0 To UBound($avMap, 1) - 1 If $avChr[$m][$l] = $avMap[$r][0] Then ExitLoop EndIf Next $charX = $avMap[$r][1] $Len = StringLen($charX) If $m = 0 And $l = 0 And $avResults[0][0] = "" Then $j = 0 Else If $m >= 0 And $l = 0 Then $j = UBound($avResults) ReDim $avResults[UBound($avResults) + 1][13] EndIf EndIf If $charX = "X" Then $avResults[$j][0] = $avDbNF[$m] $e += 1 ContinueLoop Else $c = 1 For $d = 0 To $Len - 1 $char2add = StringMid($charX, $c , 1) Switch $l Case 0 $possible = $char2add & $avChr[$m][1] & $avChr[$m][2] If $d = 0 Then $avResults[$j][$e] = $avDbNF[$m] $avResults[$j][$e+1] = $possible $e += 1 EndIf Case 1 $possible = $avChr[$m][0] & $char2add & $avChr[$m][2] Case 2 $possible = $avChr[$m][0] & $avChr[$m][1] & $char2add EndSwitch $avResults[$j][$e] = $possible $e += 1 $c += 1 Next EndIf Next Next EndFunc ;; ==> _ChkMap() Func _Results() If UBound($avDb, 1) = 1 And $avDb[0][0] = "" Then $DbTotal = 0 Else $DbTotal = UBound($avDb, 1) EndIf If UBound($avDbNF, 1) =1 And $avDbNF[0] = "" Then $NFTotal = 0 Else $NFTotal = UBound($avDbNF, 1) EndIf Local $a_PartsWidth[3] = [165, 280, -1] Local $a_PartsText[3] = [$DbTotal & " total records in database", UBound($avResults, 1) & " values queried", $NFTotal & " values not in database"] $GUI = GUICreate("Review: Final Results", 585, 158) $ListView0 = GUICtrlCreateListView("Original|Replacements", 10, 10, 565, 21, $LVS_NOSORTHEADER) $ListView1 = GUICtrlCreateListView("0|1|2|3|4|5|6|7|8|9|10|11|12", 10, 29, 565, 106, BitOR($LVS_NOCOLUMNHEADER, $LVS_SORTASCENDING)) $StatusBar1 = _GUICtrlStatusBarCreate($GUI, $a_PartsWidth, $a_PartsText) GUISetState(@SW_SHOW) For $i = 0 To UBound($avResults, 1) -1 $lv_item = GUICtrlCreateListViewItem($avResults[$i][0] & "|" & $avResults[$i][1] & "|" &$avResults[$i][2] _ & "|" &$avResults[$i][3] & "|" &$avResults[$i][4] & "|" &$avResults[$i][5] & "|" &$avResults[$i][6] _ & "|" &$avResults[$i][7] & "|" &$avResults[$i][8] & "|" &$avResults[$i][9] & "|" &$avResults[$i][10] _ & "|" &$avResults[$i][11] & "|" &$avResults[$i][12], $ListView1) Next _GUICtrlListViewSetColumnWidth($ListView0, 0, 65) _GUICtrlListViewSetColumnWidth($ListView0, 1, 496) _GUICtrlListViewSetColumnWidth($ListView1, 0, 65) For $i = 1 To 12 _GUICtrlListViewSetColumnWidth($ListView1, $i, 40) Next ToolTip("Close window to write query to the database " & @CRLF & 'file "HexDb.csv" and exit script', 925, 295, "Close Results", 1, 1) While 1 $Mouse = MouseGetPos() If $Mouse[0] > 345 And $Mouse[0] < 954 And $Mouse[1]> 285 And $Mouse[1]< 311 Then ToolTip("") EndIf $msg = GUIGetMsg() If $msg = $GUI_EVENT_CLOSE Then ExitLoop Sleep(100) Wend GUIDelete() If $Lctrl <> "no-change" Then _WriteOutDb() EndFunc ;; ==> _Results() Func _WriteOutDb() $file = FileOpen("HexDb.csv", 1) For $m = 0 To UBound($avResults, 1) -1 For $r = 0 To UBound($avDbNF, 1) -1 If $avResults[$m][0] = $avDbNF[$r] Then $sWresults = $avResults[$m][0] & "," For $c = 1 To UBound($avResults, 2) - 1 If $avResults[$m][$c] <> "" Then $sWresults &= $avResults[$m][$c] & "," Next $sWresults = StringTrimRight($sWresults, 1) FileWriteLine($file, $sWresults) ExitLoop EndIf Next Next FileClose($file) _ReadInDb() _ArraySort($avDb,0,0,0,13) FileMove("HexDb.csv", "HexDb.csv.bak", 1) $Lctrl = "sort" _DbHeader() For $m = 0 To UBound($avDb, 1) -1 $sWresults = "" For $c = 0 To UBound($avDb, 2) - 1 If $avDb[$m][$c] <> $avDb[UBound($avDb, 1) -1][UBound($avDb, 2) -1] Then If $avDb[$m][$c] <> "" Then $sWresults &= $avDb[$m][$c] & "," Endif Next $sWresults = StringTrimRight($sWresults, 1) FileWriteLine($file, $sWresults) Next FileClose($file) EndFunc ;; ==> _WriteOutDb() Func _DbHeader() If $Lctrl = "sort" Then $file = FileOpen("HexDb.csv", 2) Else $file = FileOpen("HexDb.csv", 1) EndIf FileWriteLine($file, "HexDb.csv serves as the database for HexReplacements.au3 which was") FileWriteLine($file, "inspired by Czardas's Zero2One post (http://preview.tinyurl.com/2zyunc).") FileWriteLine($file, "...........") FileWriteLine($file, "R0 through R12 are the possible replacements for each value.") FileWriteLine($file, "...........") FileWriteLine($file, "Value,R0,R1,R2,R3,R4,R5,R6,R7,R8,R9,R10,R11,R12") EndFunc ;; ==< _DbHeader() Func _Exit() $errExit = MsgBox(4+32+0+262144,"Exit Script", "Do you want to exit the script?") Switch $errExit Case 7 $Lctrl = "continue" Return 0 Case 6 Exit EndSwitch EndFunc ;; ==> _Exit()
PsaltyDS Posted August 20, 2007 Posted August 20, 2007 Hi guys, I'm so stoked about this being my first au3 script that has no mouseclicks, no send commands, no winwaits... and for me that's HUGE!!! Of course I couldn't have done this without the help of PsaltyDS, big_daddy, Paulie, MHz, Siao and others schooling me on UBound and Arrays. Like PsaltyDS says, "arrays are our friends" and boy are they!Ok so what does this do? At its core its the same program Czardas put together and asked for help on this thread (http://www.autoitscript.com/forum/index.php?showtopic=50987&hl=zero2one). I was going to offer some suggestion back when he/she first posted the message, but this kind of took on a life of its own and what a learning experience this has been for me! Czardas, thanks for the idea and I hope you like my version.Summary:Takes a 3 digit hex number like 2A5 and offers the possible replacements of 3A5, 6A5, AA5, 2B5, 2E5, 2A7 and 2AD.This is different than Czardas' version in that it allows multiple entries and displays them on the screen before writing the results to a csv file. If you run the script again it will first check against the csv file for matches before moving onto the $avMap array. Also at the end it will update the csv file and sort the file.Purpose of this post:1 - To ask the code gurus to review and rate the script (keep in mind I don't have a developer background)2 - See if the code can be reduced from its 343 lines. I'm guessing SmOke_N or PsaltyDS can shrink this down to 100 or less lines.3 - After having my code torn to pieces by the gurus, I hope to learn some better practices.4 - See if anyone can find any bugs I may have over looked in my extensive testing.Well enough now for the code! Thanks again for those that helped me, I'm embarrassed to say this took me 2 weeks to write <blush>. PsaltyDS... thumbs up???Glad you got it working. I don't understand what you do with these values once they are generated (and stored in a .csv file). What do you and Czardas do with the output? Valuater's AutoIt 1-2-3, Class... Is now in Session!For those who want somebody to write the script for them: RentACoder"Any technology distinguishable from magic is insufficiently advanced." -- Geek's corollary to Clarke's law
Moderators SmOke_N Posted August 20, 2007 Moderators Posted August 20, 2007 I'm not quite sure how I got brought into this... but the script is too long and not a working example for me to go through the whole thing... but since you brought up "less" lines (not always the best method keep in mind)... playing with this one function dropped 30+ lines off of the code... from here you may get some insight on what it is doing (since you wrote the code an know anyway)... I only made one comment in that first function, and did not bother to go past that function:expandcollapse popupFunc _GetInputs() While 1 $input = InputBox("Input", "Valid hexadecimal characters are 09 and AF. Example: A0F" & _ @CRLF&@CRLF&"Enter your 3 digit hexadecimal value.") $f = StringUpper($input) If @error Then $Ectrl = "0-cancel" _Exit() $errExit = MsgBox(4+32+0+262144,"Exit Script", "Do you want to exit the script?") If $errExit = 7 And $avInput[0] <> "" Then ExitLoop If $errExit = 6 Then ExitLoop Else If $input = "" Or (StringLen($input) <> 3 Or Not StringIsXDigit($input)) Then $errExit = MsgBox(6+16+256+262144,"Error: Invalid Entry", "You did not enter a valid 3 digit hexidecimal value." _ & @CRLF & @CRLF & "Hexadecimal characters are 09 and AF." _ &@CRLF &"Examples: A0F, 2FF, 00E, BE2" & @CRLF & @CRLF) If $errExit = 11 If $avInput[0] = "" Then _Exit() $Lctrl = "continue" ExitLoop EndIf If $errExit = 2 Then _Exit() Else For $d = 0 To UBound($avInput, 1) -1 If $input = $avInput[$d] Then $errExit = MsgBox(6+16+256+262144,"Error: Duplicate Entry", $avInput[$d] & " has already been entered.") If $errExit = 11 Then $Lctrl = "continue" If $errExit = 10 Then $Lctrl = "retry" If $errExit = 2 Then _Exit() EndIf Next If $Lctrl = "retry" Then $Lctrl = "" If $Lctrl = "continue" Then $Lctrl = "" ExitLoop Else If $c = 0 Then _ArrayPush($avInput, $f) Else _ArrayAdd($avInput, $f) EndIf $results = MsgBox(4+32+0+262144, "Accepted: " & $avInput[$c], "Do you have more values to enter?") $c += 1 If $results = 7 Then ExitLoop EndIf EndSwitch EndSwitch Wend ;I would make UBound($avInput) a Local variable so I only had to call it once ie... ;Local $myUbound = UBound($avInput) ... now you don't have to make the 5 or 6 calls to it below (saving time) $msg = "The follow " & UBound($avInput, 1) & " values will be processed to " & _ @CRLF & "determine their replacement values." If UBound($avInput, 1) -1 > 6 Then $sInputs = _ArrayToString( $avInput, ", ", 0, 4) $summary = MsgBox(1+64+0+262144, "Review: Input", $msg &@CRLF & @CRLF & $sInputs & ", ... " & $avInput[UBound($avInput, 1) - 1]) ElseIf UBound($avInput, 1) -1 = 0 Then $summary = MsgBox(1+64+0+262144, "Review: Input", $msg &@CRLF & @CRLF & $avInput[0]) Else $sInputs = _ArrayToString( $avInput, ", ", 0, UBound($avInput, 1) - 1) $summary = MsgBox(1+64+0+262144, "Review: Input", $msg &@CRLF & @CRLF & $sInputs) EndIf If $summary = 2 Then _Exit() EndIf EndFunc ;; ==> _GetInputs() Common sense plays a role in the basics of understanding AutoIt... If you're lacking in that, do us all a favor, and step away from the computer.
ssubirias3 Posted August 21, 2007 Author Posted August 21, 2007 @PsaltyDS - I don't know what Czardas plans are/were for the output. All I know is there were some inefficiencies that would further support the idea of using autoit in the first place. It didn't make sense to me to query only one 3 digit value for its possible replacements and never see it until you manually opened the add.log file. Moreover the idea of having the output append to the add.log to potential lose which sets of replacments belong to what value... i just saw an add.log file that might not prove useful as it grew or was overwritten. So from my perspective, I was stretching myself by trying to fill the gaps that I saw in the Czardas' version. Not so much to do this for him/her, but rather to solidify and expand my new found knowledge of arrays and ubound. And because of this exercise, I'm now much more confident about using 1d and 2d arrays. @SmOke_N' thanks for glancing over the first function and offering your suggestions! Good point about "less" not always being the best method. Since many of the "tricks of the trade" are learned through years of coding, study, and/or networking, I was asking folks such as yourself to share some best practices. Since I have your attention, can you or someone offer a suggestion on how to consolidate the following code into one conditional statement? If UBound($avDb, 1) = 1 And $avDb[0][0] = "" Then $DbTotal = 0 Else $DbTotal = UBound($avDb, 1) EndIf If UBound($avDbNF, 1) =1 And $avDbNF[0] = "" Then $NFTotal = 0 Else $NFTotal = UBound($avDbNF, 1) EndIf Again thanks for the feedback, I'm just trying to learn better techniques from the people who have been writing code for several years.
Moderators SmOke_N Posted August 21, 2007 Moderators Posted August 21, 2007 You have 4 different conditions to meet, and you want "1" conditional statement? The request is so broad, without any real output request that no one could make your request happen without guessing. We don't even know if the conditions rely on one another as an example. Common sense plays a role in the basics of understanding AutoIt... If you're lacking in that, do us all a favor, and step away from the computer.
ssubirias3 Posted August 21, 2007 Author Posted August 21, 2007 You have 4 different conditions to meet, and you want "1" conditional statement? The request is so broad, without any real output request that no one could make your request happen without guessing. We don't even know if the conditions rely on one another as an example. :"> Opps! Silly me, I should have mentioned the two conditional statements were from the _Results function and clarified that their purpose was for aesthetics; the gui status bar and final exit message. Func _Results() If UBound($avDb, 1) = 1 And $avDb[0][0] = "" Then $DbTotal = 0 Else $DbTotal = UBound($avDb, 1) EndIf If UBound($avDbNF, 1) =1 And $avDbNF[0] = "" Then $NFTotal = 0 Else $NFTotal = UBound($avDbNF, 1) EndIf Local $a_PartsWidth[3] = [165, 280, -1] Local $a_PartsText[3] = [$DbTotal & " total records in database", UBound($avResults, 1) & " values queried", $NFTotal & " values not in database"]oÝ÷ ØX§jQ1Ó²Æ y«¢+Ù5Í ½à ÈØÈÈÀà°ÅÕ½Ðíá¥ÐMÉ¥ÁÐÅÕ½Ðì°ÅÕ½ÐíQ¡ÍÉ¥ÁСÌÅÕ½ÐìµÀìÀÌØí9Q½Ñ°µÀìÅÕ½ÐìɽÉÌÅÕ½ÐìµÀì|( I1µÀìÅÕ½Ðí¹Í½ÉÑÑ¡Ñ͸ÅÕ½Ðì¤oÝ÷ Ù8ZK-£*.ø«²Úh²Òjg®§Ø^~e£§ÈZzÊbµæ趮ø§Ø^¦êé¢Çv÷öÙ'£¶«{l²Ø¥ª®¢Ö޶׫Á¬ªºh}Ú"aÄájy,j¢ú+Ê«¥«bzwjwpYb Þ²Ëhéijz(nX¤zgDãºÚ"µÍ[ÈÔÝ[Ê BYPÝ[ ÌÍØ]JHHH[ ÌÍØ]ÌVÌHH ][ÝÉ][ÝÈ[ ÌÍÑÝ[H[ÙB ÌÍÑÝ[HPÝ[ ÌÍØ]JB[YYPÝ[ ÌÍØ]JHLH[ ÌÍØ]ÌHH ][ÝÉ][ÝÈ[ ÌÍÓÝ[H[ÙB ÌÍÓÝ[HPÝ[ ÌÍØ]JB[YoÝ÷ Ù3ºÚ"µÍ[ÈÔÝ[Ê B ÌÍÑÝ[HPÝ[ ÌÍØ]JB ÌÍÓÝ[HPÝ[ ÌÍØ]JBYPÝ[ ÌÍØ]JHOHH[ ÌÍØ]ÌVÌHOH ][ÝÉ][ÝÈ[ ÌÍÑÝ[HYPÝ[ ÌÍØ]JHOHH[ ÌÍØ]ÌHOH ][ÝÉ][ÝÈ[ ÌÍÓÝ[H Some may think, "if it works its good," but I don't believe working code is necessarily "good code." I want to learn the good coding practices before bad habits become harder to break out of. That's really what's at the heart of this thread. Just looking to see where the fat, inefficiencies, unnecessary code, etc can be trimmed. Hope that makes sense.
Moderators SmOke_N Posted August 21, 2007 Moderators Posted August 21, 2007 This was an interesting link: http://www.zuskin.com/coding_habits.htmOne thing that may help you, that helped me (AutoIt being my first attempt at any type of coding/scripting), is when you learn something, try to give back to the community. When you see a plea for help, attempt to work out a solution for it, then post your solution.After seeing your code up there, you may see others code as well to accomplish the same thing, look at the differences.... It can be quite a humbling experience, but if you look at it the right way, you've accomplished 2 things. 1. You've given the OP (original poster) an option to look at that they had not had, and 2. You may see how to accomplish such things much easier for future community help and or your own projects.I would stay away from trying to structure my code around others that have only used AutoIt in the past (1st language... Myself included). I took a liking to the way that Jdeb coded, when I was writing a piece of code at one time, his out look on string manipulation was inspiring... It was amazing he could accomplish in a few lines what took me a hundred too... so I tried to mold my coding style after his... and eventually came up with my own that works for me. I guess what I'm saying, is find a mentor, and when you feel comfortable with understanding the way their code flows, then mold yourself after that person until you can find your own coding style. People like Jdeb,Valik,Larry,Gafrost,MHz,Dale, the list could go on, really know their stuff, and if you read their workings, you'll see a very nice "efficient" coding pattern (unique even) to each of them.Anyway, I commend you on your eagerness, and hope to see you succeed in your endeavor for excellence.(P.S. - I started to ramble there a bit, hope you got the gist of what I was talking about) Common sense plays a role in the basics of understanding AutoIt... If you're lacking in that, do us all a favor, and step away from the computer.
ssubirias3 Posted August 21, 2007 Author Posted August 21, 2007 (edited) This was an interesting link: http://www.zuskin.com/coding_habits.htm One thing that may help you, that helped me (AutoIt being my first attempt at any type of coding/scripting), ....Thanks SmoKe_N, much of what you outlined is what I have done and am in the process of doing, hence the title of the thread "Calling all gurus" I've learked on the forum for about a year and only started applying myself to scripting. I have a list of names who appear to know what they are doing and the common denominator is the MVP status. Granted, some politics could play into who's MVP or not, and all MVP's styles might not blend with my personality... but that's life, right??! I had no idea AutoIt was your first language and that makes my theory of "bringing you into this" even more founded. I would think you could appreciate where I am now since you were once in my shoes. Knowing that you started with AutoIt like I am is very inspiring since you are where you are today. And of couse thank you for the link. That's exactly the kind of stuff you and others take for granted you know that you know know you know and more importantly don't know that we [noobs] don't know. Sure that's what google and the like are for, however how is a noob to know any of the hits are "worth the time." So this link being SmoKe_N' approved you know its gotta be good! Thanks again for your time and suggestions. I may have to ask Valik for his thoughts, from the posts I've read I think there's a small kindred spirit between Valik, PsaltyDS, and you. Then again I've got a crazy imagination. Edit: Just looked over the link, and dats exactly what I'm talking about... Ahh is life good or what!! Now THIS is starting to feel more like Help and Support should, imo. In fact because of that link I've taken the following code snippet FROM: If UBound($avDb, 1) = 1 And $avDb[0][0] = "" Then $DbTotal = 0 Else $DbTotal = UBound($avDb, 1) EndIf If UBound($avDbNF, 1) =1 And $avDbNF[0] = "" Then $NFTotal = 0 Else $NFTotal = UBound($avDbNF, 1) EndIfoÝ÷ Ù3ºÚ"µÍYÝ PÝ[ ÌÍØ]JHOHH[ ÌÍØ]ÌVÌHOH ][ÝÉ][ÝÊH[ ÌÍÑÝ[HPÝ[ ÌÍØ]JBYÝ PÝ[ ÌÍØ]JHOHH[ ÌÍØ]ÌHOH ][ÝÉ][ÝÊH[ ÌÍÓÝ[HPÝ[ ÌÍØ]J Edited August 21, 2007 by ssubirias3
weaponx Posted August 21, 2007 Posted August 21, 2007 (edited) Well I took you up on optimizing your script. This version isn't yet complete because it's almost 5:00 and I haven't done any work today, but maybe I will look at it tomorrow. expandcollapse popup#include <Array.au3> #include <File.au3> Const $CSV = "HexDb.csv" Const $CSVOUT = "test.csv" Dim $DBContents Dim $FinalArray[1][2] Dim $avMap[16][2] = [["A","BE"],["B","F"],["C","DE"],["D","F"],["E","F"],["F","X"],["0","1248"],["1","359"],["2","36A"],["3","7B"],["4","56C"],["5","7D"],["6","7E"],["7","F"],["8","9AC"],["9","BD"]] ;Get new values $input = InputBox("Input", "Valid hexadecimal characters are 0–9 and A–F. Example: A0F" & @CRLF & @CRLF & "Enter 3 digit hexadecimal values (comma seperated).") $hexArray = StringSplit ( $input, ",") For $K = 1 to $hexArray[0] If $K <> 1 Then Redim $FinalArray[Ubound($FinalArray) + 1][2] ;Verify hex code is entered If NOT StringIsXDigit ( $hexArray[$K] ) Then MsgBox(0,"","Element: " & $K & " Value: " & $hexArray[$K] & " is invalid hex code") ;Limit length to 3 characters $hexArray[$K] = StringLeft($hexArray[$K], 3) ;Strip duplicates from array $result = _ArraySearch ($hexArray, $hexArray[$K], 1, 0, 0, False ) If NOT @ERROR AND $result <> $K Then _ArrayDelete ( $hexArray, $result) ;Add to final array $FinalArray[Ubound($FinalArray) -1][0] = $hexArray[$K] Next ;MsgBox(0,"","The following " & $hexArray[0] & " values will be processed to determine their replacement values." & @CRLF & _ArrayToString ( $hexArray, ",", 1)) ;Read in contents of CSV file if it exists If FileExists ( $CSV ) Then _FileReadToArray ( $CSV, $DBContents ) For $K = 7 to $DBContents[0] $OLDCodes = StringSplit ($DBContents[$K], ",") ;Add to final array If StringIsXDigit ($OLDCodes[1]) AND $OLDCodes[1] <> "" Then ;Search user entered hex codes for duplicates $result = _ArraySearch ($hexArray, $OLDCodes[1], 1, 0, 0, False ) If @ERROR Then ;Add to final array Redim $FinalArray[Ubound($FinalArray) + 1][2] $FinalArray[Ubound($FinalArray) - 1][0] = $OLDCodes[1] EndIf EndIf Next EndIf ;Loop through final array (User entered codes + CSV codes merged) For $X = 0 to Ubound($FinalArray) - 1 $SplitCode = StringSplit( $FinalArray[$X][0], "") ;Loop through each of the 3 characters For $K = 1 to $SplitCode[0] ;Search replacement map matrix 0-9, A-F For $V = 0 to Ubound($avMap) - 1 ;If current character matches If $avMap[$V][0] = $SplitCode[$K] Then ;Break swap code string into array $swapChars = StringSplit ( $avMap[$V][1], "") ;Loop through sub items of replacement map For $Z = 1 to $swapChars[0] ;Copy current hex character array to temp array, replace current character with one from replacement mapping $tempArray = $SplitCode ;If map is X, replace with current character If $swapChars[$Z] = "X" Then $tempArray[$K] = $SplitCode[$K] Else $tempArray[$K] = $swapChars[$Z] EndIf $FinalArray[$X][1] &= _ArrayToString ($tempArray, "", 1) & "," Next EndIf Next Next Next ;_ArrayDisplay($FinalArray) ;Sort Final array _ArraySort ($FinalArray,0,0,0,2) _ArrayDisplay($FinalArray) ;Write to CSV $file = FileOpen($CSVOUT, 2) FileWriteLine($file, "HexDb.csv serves as the database for HexReplacements.au3 which was") FileWriteLine($file, "inspired by Czardas's Zero2One post (http://preview.tinyurl.com/2zyunc).") FileWriteLine($file, "...........") FileWriteLine($file, "R0 through R12 are the possible replacements for each value.") FileWriteLine($file, "...........") FileWriteLine($file, "Value,R0,R1,R2,R3,R4,R5,R6,R7,R8,R9,R10,R11,R12") For $K = 0 to Ubound($FinalArray) - 1 FileWriteLine($file, $FinalArray[$K][0] & "," & $FinalArray[$K][1]) Next EDIT: It's actually pretty complete now. Will read in HexDb.csv if it exists but will output to test.csv Edited August 21, 2007 by weaponx
ssubirias3 Posted August 21, 2007 Author Posted August 21, 2007 Well I took you up on optimizing your script.This version isn't yet complete because it's almost 5:00 and I haven't done any work today, but maybe I will look at it tomorrow.... CODE: AutoIt ...EDIT: It's actually pretty complete now. Will read in HexDb.csv if it exists but will output to test.csvWoah, taking a look over the changes this is going to be fun! I haven't read through everything yet to see the difference and understand the why yet, but I'm pretty excited this will give me a new prespective and style to consider. And its under 100 lines of code if you take out the comments! That also without the multiple prompts and final gui. And I like the idea of using $hexArray = StringSplit ( $input, ",") to allow multiple values to be entered at one time. I had the idea, just put it on the back burner since I wanted to finish then review my own inefficiencies. Also noticed that FFF returns 3 possibilities all being FFF where my original code return nothing. Same difference I guess since FFF with no replacements still leaves you with only itself as an option.Thanks weaponx, I appreciate you lending a hand!
weaponx Posted August 22, 2007 Posted August 22, 2007 I know my code is going to be great when it looks like the Millenium Force coaster on its side.
ssubirias3 Posted August 27, 2007 Author Posted August 27, 2007 (edited) Purpose of this post: 1 - To ask the code gurus to review and rate the script (keep in mind I don't have a developer background) 2 - See if the code can be reduced from its 343 lines. I'm guessing SmOke_N or PsaltyDS can shrink this down to 100 or less lines. 3 - After having my code torn to pieces by the gurus, I hope to learn some better practices. 4 - See if anyone can find any bugs I may have over looked in my extensive testing.Thanks to SmOke_N and weaponx' ideas I've been able to whittle down the code from its original 343 lines to 229 lines. That's a 33% reduction! Much of this was accomplished by streamlining the _GetInput function. Im still interested in other more efficient ways of working with the data and multiple arrays and functions within this script. Thanks in advance. expandcollapse popup#include <array.au3> #include <GUIConstants.au3> #Include <GuiListView.au3> #Include <GuiStatusBar.au3> Opt('GUICloseOnESC', 1) Dim $avInput[1], $avValid[1], $avChr[1][3], $avDb[1][13], $avResults[1][13], $avDbNF[1], $avTemp[1] Dim $file = "HexDb.csv", $lnumb = 5, $c = 0, $Lctrl, $DbTotal = 0, $NFTotal = 0, $xyz _GetInputs() _ReadInDb() _ChkDb() _Results() MsgBox(262208, "Exit Script", "The script has added " & $NFTotal & " records " & _ @CRLF & "and sorted the database.") Func _GetInputs() $input = StringUpper(InputBox("Input", "Valid hexadecimal characters are 0–9 and A–F. Example: A0F, 2FF, 00E, BE2" & _ @CRLF & @CRLF & 'Enter 3 digit hexadecimal values seperated by a ",".')) $avInputs = StringRegExp($input, '(\w+?)(?m:$|,)', 3) If @error Then _Exit() Else For $i = 0 To UBound($avInputs, 1) - 1 If StringLen($avInputs[$i]) == 3 And StringIsXDigit($avInputs[$i]) Then For $d = 0 To UBound($avValid, 1) - 1 If $avInputs[$i] == $avValid[$d] Then ContinueLoop 2 EndIf Next _ArrayAdd($avValid, $avInputs[$i]) EndIf Next _ArrayDelete($avValid, 0) EndIf $msg = "The follow " & UBound($avValid, 1) & " values will be processed to " & _ @CRLF & "determine their replacement values." & @CRLF & @CRLF $sInputs = _ArrayToString($avValid, ", ", 0, UBound($avValid, 1) - 1) If UBound($avValid, 1) - 1 > 6 Then $sInputs = _ArrayToString($avValid, ", ", 0, 4) & _ " ... " & $avValid[UBound($avValid, 1) - 1] If UBound($avValid, 1) - 1 = 0 Then $sInputs = $avValid[0] $summary = MsgBox(262209, "Review: Input", $msg & $sInputs) If $summary == 2 Then _Exit() EndFunc ;==>_GetInputs Func _ReadInDb() If FileExists($file) Then While 1 $line = FileReadLine($file, $lnumb) If @error == -1 Then ExitLoop $avTemp = StringSplit($line, ",") For $c = 1 To $avTemp[0] $avDb[$lnumb - 5][$c - 1] = $avTemp[$c] Next ReDim $avDb[UBound($avDb, 1) + 1][13] $lnumb += 1 WEnd EndIf FileClose($file) EndFunc ;==>_ReadInDb Func _ChkDb() Dim $c = 0, $d = 0 For $m = 0 To UBound($avValid, 1) - 1 $Lctrl = "no-match" For $r = 0 To UBound($avDb, 1) - 1 If $avValid[$m] == $avDb[$r][0] Then $Lctrl = "match" ExitLoop EndIf Next If $Lctrl == "match" Then If $d == 0 Then For $i = 0 To UBound($avDb, 2) - 1 $avResults[$d][$i] = $avDb[$r][$i] Next Else ReDim $avResults[UBound($avResults) + 1][13] For $i = 0 To UBound($avDb, 2) - 1 $avResults[$d][$i] = $avDb[$r][$i] Next EndIf $d += 1 Else If $c == 0 Then _ArrayPush($avDbNF, $avValid[$m]) Else _ArrayAdd($avDbNF, $avValid[$m]) EndIf $c += 1 EndIf Next $Lctrl = "no-change" If $avDbNF[0] <> "" Then _ChkMap() EndFunc ;==>_ChkDb Func _ChkMap() Local $avMap[16][2] = [["A", "BE"], ["B", "F"], ["C", "DE"], ["D", "F"], ["E", "F"], ["F", "X"], ["0", "1248"] _ , ["1", "359"], ["2", "36A"], ["3", "7B"], ["4", "56C"], ["5", "7D"], ["6", "7E"], ["7", "F"], ["8", "9AC"], ["9", "BD"]] For $i = 0 To UBound($avDbNF, 1) - 1 For $j = 0 To UBound($avChr, 2) - 1 $avTemp = StringSplit($avDbNF[$i], "") $avChr[$i][$j] = $avTemp[$j + 1] Next If $i <> UBound($avDbNF, 1) - 1 Then ReDim $avChr[UBound($avChr, 1) + 1][3] Next For $m = 0 To UBound($avDbNF, 1) - 1 $e = 0 For $l = 0 To 2 For $r = 0 To UBound($avMap, 1) - 1 If $avChr[$m][$l] == $avMap[$r][0] Then ExitLoop Next $charX = $avMap[$r][1] $Len = StringLen($charX) If $m == 0 And $l == 0 And $avResults[0][0] == "" Then Dim $j = 0, $xyz = 0 ElseIf $m == 0 And $l == 0 Then Dim $j = UBound($avResults), $xyz = UBound($avResults) ReDim $avResults[UBound($avResults) + 1][13] Else If $m >= 1 And $l = 0 Then $j = UBound($avResults) ReDim $avResults[UBound($avResults) + 1][13] EndIf EndIf If $charX == "X" Then $avResults[$j][0] = $avDbNF[$m] $e += 1 ContinueLoop Else $c = 1 For $d = 0 To $Len - 1 $char2add = StringMid($charX, $c, 1) Switch $l Case 0 $possible = $char2add & $avChr[$m][1] & $avChr[$m][2] If $d == 0 Then $avResults[$j][$e] = $avDbNF[$m] $avResults[$j][$e + 1] = $possible $e += 1 EndIf Case 1 $possible = $avChr[$m][0] & $char2add & $avChr[$m][2] Case 2 $possible = $avChr[$m][0] & $avChr[$m][1] & $char2add EndSwitch $avResults[$j][$e] = $possible $e += 1 $c += 1 Next EndIf Next Next $Lctrl = "" EndFunc ;==>_ChkMap Func _Results() If Not (UBound($avDb, 1) == 1 And $avDb[0][0] == "") Then $DbTotal = UBound($avDb, 1) If Not (UBound($avDbNF, 1) == 1 And $avDbNF[0] == "") Then $NFTotal = UBound($avDbNF, 1) Local $a_PartsWidth[3] = [165, 280, -1] Local $a_PartsText[3] = [$DbTotal & " total records in database", UBound($avResults, 1) & " values queried", $NFTotal & " values not in database"] $GUI = GUICreate("Review: Final Results", 585, 158) $ListView0 = GUICtrlCreateListView("Original|Replacements", 10, 10, 565, 21, $LVS_NOSORTHEADER) $ListView1 = GUICtrlCreateListView("0|1|2|3|4|5|6|7|8|9|10|11|12", 10, 29, 565, 106, BitOR($LVS_NOCOLUMNHEADER, $LVS_SORTASCENDING)) $StatusBar1 = _GUICtrlStatusBarCreate($GUI, $a_PartsWidth, $a_PartsText) GUISetState(@SW_SHOW) For $i = 0 To UBound($avResults, 1) - 1 $lv_item = GUICtrlCreateListViewItem($avResults[$i][0] & "|" & $avResults[$i][1] & "|" & $avResults[$i][2] _ & "|" & $avResults[$i][3] & "|" & $avResults[$i][4] & "|" & $avResults[$i][5] & "|" & $avResults[$i][6] _ & "|" & $avResults[$i][7] & "|" & $avResults[$i][8] & "|" & $avResults[$i][9] & "|" & $avResults[$i][10] _ & "|" & $avResults[$i][11] & "|" & $avResults[$i][12], $ListView1) Next _GUICtrlListViewSetColumnWidth($ListView0, 0, 65) _GUICtrlListViewSetColumnWidth($ListView0, 1, 496) _GUICtrlListViewSetColumnWidth($ListView1, 0, 65) For $i = 1 To 12 _GUICtrlListViewSetColumnWidth($ListView1, $i, 40) Next ToolTip("Close window to write query to the database " & @CRLF & 'file "HexDb.csv" and exit script', 925, 295, "Close Results", 1, 1) While 1 Dim $msg = GUIGetMsg(), $Mouse = MouseGetPos() If $msg == $GUI_EVENT_CLOSE Then ToolTip("") ExitLoop EndIf If $Mouse[0] > 345 And $Mouse[0] < 954 And $Mouse[1] > 285 And $Mouse[1] < 311 Then ToolTip("") If $Mouse[0] > 925 And $Mouse[0] < 1174 And $Mouse[1] > 310 And $Mouse[1] < 383 Then ToolTip("") Sleep(40) WEnd GUIDelete() If $Lctrl <> "no-change" Then _WriteOutDb() EndFunc ;==>_Results Func _WriteOutDb() For $g = $xyz To UBound($avResults, 1) - 1 $klm = UBound($avDb, 1) - 1 For $h = 0 To UBound($avResults, 2) - 1 $avDb[$klm][$h] = $avResults[$g][$h] Next If $g <> UBound($avResults, 1) - 1 Then ReDim $avDb[UBound($avDb, 1) + 1][13] Next _ArraySort($avDb, 0, 0, 0, 13) FileMove($file, "HexDb.csv.bak", 1) $file = FileOpen($file, 2) FileWriteLine($file, "HexDb.csv serves as the database for HexReplacements.au3.") FileWriteLine($file, "R0 through R12 are the possible replacements for each value.") FileWriteLine($file, "...........") FileWriteLine($file, "Value,R0,R1,R2,R3,R4,R5,R6,R7,R8,R9,R10,R11,R12") For $m = 0 To UBound($avDb, 1) - 1 $sWresults = "" For $c = 0 To UBound($avDb, 2) - 1 If $avDb[$m][$c] <> $avDb[UBound($avDb, 1) - 1][UBound($avDb, 2) - 1] Then If $avDb[$m][$c] <> "" Then $sWresults &= $avDb[$m][$c] & "," EndIf Next $sWresults = StringTrimRight($sWresults, 1) FileWriteLine($file, $sWresults) Next FileClose($file) EndFunc ;==>_WriteOutDb Func _Exit() $errExit = MsgBox(4 + 32 + 0 + 262144, "Exit Script", "Do you want to exit the script?") If ($errExit == 7) Or ($errExit == 7 And $avValid[0] <> "") Then Return 0 If $errExit == 6 Then Exit EndFunc ;==>_Exit Edit: Corrected percentage, Thanks weaponx! Edited August 27, 2007 by ssubirias3
weaponx Posted August 27, 2007 Posted August 27, 2007 Actual percentage decrease in code: 33.23% ((343 - 229) / 343) * 100
Recommended Posts
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 accountSign in
Already have an account? Sign in here.
Sign In Now