Rimoun Posted June 21, 2017 Posted June 21, 2017 Hello everyone I would like to have some help regarding my issue. I am trying to extract some information from excel sheet, I use _Excel_RangeFind then I get the array for the value then I check the array. In case the array is true the next will be to get some information based on the extracted array if no array because the value is not found it gives a messages box that the value is not found. My problem that the button works for one time only if i tried any value which exist in the sheet it gives me that value not found. I suspect that there is a problem regarding While loop. here is my code expandcollapse popup#include <GUIConstantsEx.au3> #include <WinAPI.au3> #Include <GuiListBox.au3> #include <WindowsConstants.au3> #include <Array.au3> #include <Excel.au3> #include <MsgBoxConstants.au3> #include <ExcelConstants.au3> if FileExists ("result.txt") Then Sleep (100) Else readxl() EndIf Global $oExcel = _Excel_Open(False,False) If @error Then Exit MsgBox($MB_SYSTEMMODAL, "Excel UDF: _Excel_RangeFind Example", "Error creating the Excel application object." & @CRLF & "@error = " & @error & ", @extended = " & @extended) Global $oWorkbook = _Excel_BookOpen($oExcel, "D:\info.xlsx",False,False) If @error Then MsgBox($MB_SYSTEMMODAL, "Excel Error", "Error opening workbook '" & $oWorkbook & @CRLF & "@error = " & @error & ", @extended = " & @extended) _Excel_Close($oExcel) Exit EndIf Local $lab1 = _Excel_RangeRead($oWorkbook, Default,"B1") Local $lab2 = _Excel_RangeRead($oWorkbook, Default,"C1") Local $lab3 = _Excel_RangeRead($oWorkbook, Default,"D1") Local $lab4 = _Excel_RangeRead($oWorkbook, Default,"E1") Global $sResult1,$sResult2,$sResult3,$sResult4,$sResult5,$sResult6 Global $asKeyWords = stringsplit (FileRead (@ScriptDir & "\result.txt"),@CRLF) Global Const $xlUp = -4162 ;~ _Main() Local $hGUI, $hList, $hInput, $aSelected, $sChosen, $hUP, $hDOWN, $hENTER, $hESC Local $sCurrInput = "", $aCurrSelected[2] = [-1, -1], $iCurrIndex = -1, $hListGUI = -1 $hGUI = GUICreate("Rimo System", 253, 270, 192, 124) Global $hInput = GUICtrlCreateInput("", 24, 48, 169, 21) Global $Label1 = GUICtrlCreateLabel("Rimo System", 80, 16, 150, 25) GUICtrlSetFont(-1, 14, 800, 0, "MS Serif") Global $Input2 = GUICtrlCreateInput("", 72, 144, 161, 21) Global $Input3 = GUICtrlCreateInput("", 72, 176, 161, 21) Global $Input4 = GUICtrlCreateInput("", 72, 208, 161, 21) Global $Input5 = GUICtrlCreateInput("", 72, 240, 161, 21) $Input6 = GUICtrlCreateInput("", 72, 272, 161, 21) $Input7 = GUICtrlCreateInput("", 72, 304, 161, 21) $Button1 = GUICtrlCreateButton("Get Info", 72, 88, 89, 33) $Label2 = GUICtrlCreateLabel("Label2", 16, 144, 36, 17) GUICtrlSetData( -1,$lab1) $Label3 = GUICtrlCreateLabel("Label3", 16, 176, 36, 17) GUICtrlSetData( -1,$lab2) $Label4 = GUICtrlCreateLabel("Label4", 16, 208, 36, 17) GUICtrlSetData( -1,$lab3) $Label5 = GUICtrlCreateLabel("Label5", 16, 240, 36, 17) GUICtrlSetData( -1,$lab4) $Label6 = GUICtrlCreateLabel("", 16, 272, 36, 17) $Label7 = GUICtrlCreateLabel("", 16, 304, 36, 17) $Button2 = GUICtrlCreateButton("Cancel", 112, 416, 121, 25) GUISetState(@SW_SHOW, $hGUI) Global $sSearch = guictrlread ($hInput) $hUP = GUICtrlCreateDummy() $hDOWN = GUICtrlCreateDummy() $hENTER = GUICtrlCreateDummy() $hESC = GUICtrlCreateDummy() Dim $AccelKeys[4][2] = [["{UP}", $hUP], ["{DOWN}", $hDOWN], ["{ENTER}", $hENTER], ["{ESC}", $hESC]] GUISetAccelerators($AccelKeys) While 1 Switch GUIGetMsg() Case $GUI_EVENT_CLOSE ExitLoop Case $Button2 Exit Case $Button1 Global $aResult = _Excel_RangeFind($oWorkbook, guictrlread($hInput) ,"A2:A2000") Global $aExtract = _ArrayExtract($aResult, 0, 0, 2, 2) if _elementExists($aExtract,0) Then getdata() Else MsgBox(0,"","Value Does Not Exist") EndIf Case $hESC If $hListGUI <> -1 Then ; List is visible. GUIDelete($hListGUI) $hListGUI = -1 Else ExitLoop EndIf Case $hUP If $hListGUI <> -1 Then ; List is visible. $iCurrIndex -= 1 If $iCurrIndex < 0 Then $iCurrIndex = 0 EndIf _GUICtrlListBox_SetCurSel($hList, $iCurrIndex) EndIf Case $hDOWN If $hListGUI <> -1 Then ; List is visible. $iCurrIndex += 1 If $iCurrIndex > _GUICtrlListBox_GetCount($hList) - 1 Then $iCurrIndex = _GUICtrlListBox_GetCount($hList) - 1 EndIf _GUICtrlListBox_SetCurSel($hList, $iCurrIndex) EndIf Case $hENTER If $hListGUI <> -1 And $iCurrIndex <> -1 Then ; List is visible and a item is selected. $sChosen = _GUICtrlListBox_GetText($hList, $iCurrIndex) EndIf Case $hList $sChosen = GUICtrlRead($hList) EndSwitch Sleep(10) $aSelected = _GetSelectionPointers($hInput) If GUICtrlRead($hInput) <> $sCurrInput Or $aSelected[1] <> $aCurrSelected[1] Then ; Input content or pointer are changed. $sCurrInput = GUICtrlRead($hInput) $aCurrSelected = $aSelected ; Get pointers of the string to replace. $iCurrIndex = -1 If $hListGUI <> -1 Then ; List is visible. GUIDelete($hListGUI) $hListGUI = -1 EndIf $hList = _PopupSelector($hGUI, $hListGUI, _CheckInputText($sCurrInput, $aCurrSelected)) ; ByRef $hListGUI, $aCurrSelected. EndIf If $sChosen <> "" Then GUICtrlSendMsg($hInput, 0x00B1, $aCurrSelected[0], $aCurrSelected[1]) ; $EM_SETSEL. _InsertText($hInput, $sChosen) $sCurrInput = GUICtrlRead($hInput) GUIDelete($hListGUI) $hListGUI = -1 $sChosen = "" EndIf WEnd GUIDelete($hGUI) Func _CheckInputText($sCurrInput, ByRef $aSelected) Local $sPartialData = "" If (IsArray($aSelected)) And ($aSelected[0] <= $aSelected[1]) Then Local $aSplit = StringSplit(StringLeft($sCurrInput, $aSelected[0]), " ") $aSelected[0] -= StringLen($aSplit[$aSplit[0]]) If $aSplit[$aSplit[0]] <> "" Then For $A = 1 To $asKeyWords[0] If StringLeft($asKeyWords[$A], StringLen($aSplit[$aSplit[0]])) = $aSplit[$aSplit[0]] And $asKeyWords[$A] <> $aSplit[$aSplit[0]] Then $sPartialData &= $asKeyWords[$A] & "|" EndIf Next EndIf EndIf Return $sPartialData EndFunc ;==>_CheckInputText Func _PopupSelector($hMainGUI, ByRef $hListGUI, $sCurr_List) Local $hList = -1 If $sCurr_List = "" Then Return $hList EndIf $hListGUI = GUICreate("", 280, 160, 23, 62, $WS_POPUP, BitOR($WS_EX_TOOLWINDOW, $WS_EX_TOPMOST, $WS_EX_MDICHILD), $hMainGUI) $hList = GUICtrlCreateList("", 0, 0, 170, 150, BitOR(0x00100000, 0x00200000)) GUICtrlSetData($hList, $sCurr_List) GUISetControlsVisible($hListGUI) ; To Make Control Visible And Window Invisible. GUISetState(@SW_SHOWNOACTIVATE, $hListGUI) Return $hList EndFunc ;==>_PopupSelector Func _InsertText(ByRef $hEdit, $sString) #cs Description: Insert A Text In A Control. Returns: Nothing #ce Local $aSelected = _GetSelectionPointers($hEdit) GUICtrlSetData($hEdit, StringLeft(GUICtrlRead($hEdit), $aSelected[0]) & $sString & StringTrimLeft(GUICtrlRead($hEdit), $aSelected[1])) Local $iCursorPlace = StringLen(StringLeft(GUICtrlRead($hEdit), $aSelected[0]) & $sString) GUICtrlSendMsg($hEdit, 0x00B1, $iCursorPlace, $iCursorPlace) ; $EM_SETSEL. EndFunc ;==>_InsertText Func _GetSelectionPointers($hEdit) Local $aReturn[2] = [0, 0] Local $aSelected = GUICtrlRecvMsg($hEdit, 0x00B0) ; $EM_GETSEL. If IsArray($aSelected) Then $aReturn[0] = $aSelected[0] $aReturn[1] = $aSelected[1] EndIf Return $aReturn EndFunc ;==>_GetSelectionPointers Func GUISetControlsVisible($hWnd) ; By Melba23. Local $aControlGetPos = 0, $hCreateRect = 0, $hRectRgn = _WinAPI_CreateRectRgn(0, 0, 0, 0) Local $iLastControlID = _WinAPI_GetDlgCtrlID(GUICtrlGetHandle(-1)) For $i = 3 To $iLastControlID $aControlGetPos = ControlGetPos($hWnd, '', $i) If IsArray($aControlGetPos) = 0 Then ContinueLoop $hCreateRect = _WinAPI_CreateRectRgn($aControlGetPos[0], $aControlGetPos[1], $aControlGetPos[0] + $aControlGetPos[2], $aControlGetPos[1] + $aControlGetPos[3]) _WinAPI_CombineRgn($hRectRgn, $hCreateRect, $hRectRgn, 2) _WinAPI_DeleteObject($hCreateRect) Next _WinAPI_SetWindowRgn($hWnd, $hRectRgn, True) _WinAPI_DeleteObject($hRectRgn) EndFunc Func _elementExists($array, $element) If $element > UBound($array)-1 Then Return False ; element is out of the array bounds Return True ; element is in array bounds EndFunc Func getdata() ;~ Local $sResult1 = _Excel_RangeRead($oWorkbook, Default,StringReplace(StringReplace ($aExtract[0],"$",""),"A","B")) ;~ Local $sResult2 = _Excel_RangeRead($oWorkbook, Default,StringReplace(StringReplace ($aExtract[0],"$",""),"A","C")) ;~ Local $sResult3 = _Excel_RangeRead($oWorkbook, Default,StringReplace(StringReplace ($aExtract[0],"$",""),"A","D")) ;~ Local $sResult4 = _Excel_RangeRead($oWorkbook, Default,StringReplace(StringReplace ($aExtract[0],"$",""),"A","E")) ;~ Local $sResult5 = _Excel_RangeRead($oWorkbook, Default,StringReplace(StringReplace ($aExtract[0],"$",""),"A","F")) ;~ Local $sResult6 = _Excel_RangeRead($oWorkbook, Default,StringReplace(StringReplace ($aExtract[0],"$",""),"A","G")) guictrlsetdata($Input2,$sResult1) guictrlsetdata($Input3,$sResult2) guictrlsetdata($Input4,$sResult3) guictrlsetdata($Input5,$sResult4) guictrlsetdata($Input6,$sResult5) guictrlsetdata($Input7,$sResult6) _Excel_Close($oExcel,Default,True) EndFunc Func readxl() Global $oExcel = _Excel_Open(False,False) If @error Then Exit MsgBox($MB_SYSTEMMODAL, "Excel UDF: _Excel_RangeFind Example", "Error creating the Excel application object." & @CRLF & "@error = " & @error & ", @extended = " & @extended) Global $oWorkbook = _Excel_BookOpen($oExcel, "D:\info.xlsx",False,False) LOcal Const $xlUp = -4162 With $oWorkbook.ActiveSheet ; process active sheet $oRangeLast = .UsedRange.SpecialCells($xlCellTypeLastCell) ; get a Range that contains the last used cells $iRowCount = .Range(.Cells(1, 1), .Cells($oRangeLast.Row, $oRangeLast.Column)).Rows.Count ; get the the row count for the range starting in row/column 1 and ending at the last used row/column $iLastCell = .Cells($iRowCount + 1, "B").End($xlUp).Row ProgressOn("Copying Cells", "Copying Cells progress", "0%") For $i = 2 to $iLastCell Local $total = Int(($i/$iLastCell)*100) Local $sResult3 = _Excel_RangeRead($oWorkbook, Default, "A" & $i) FileWriteLine("result.txt",$sResult3) ProgressSet(($i/$iLastCell)*100, $total & "%") Next ;~ FileWrite("result.txt",$sResult3) ProgressSet(100, "Done", "Complete") Sleep (1500) ProgressOff() _Excel_Close($oExcel,Default,True) EndWith EndFunc Â
benners Posted June 21, 2017 Posted June 21, 2017 Do you have an excel sheet to try this with?, make sure there is no personal\private info inside. Some suggestions about the code. Don't declare Globals in functions, or locals in loops. A lot of the variable have already been declared and you are declaring the again with the same values. Rimoun 1
benners Posted June 21, 2017 Posted June 21, 2017 Can you post an excel sheet so I can try it with your code?. Â It's hard to try and pinpoint a reason for your button issue
Danp2 Posted June 21, 2017 Posted June 21, 2017 Why are you closing Excel at the bottom of the getdata function? Latest Webdriver UDF Release Webdriver Wiki FAQs
benners Posted June 21, 2017 Posted June 21, 2017 19 minutes ago, Danp2 said: Why are you closing Excel at the bottom of the getdata function? I think it's just that the code needs tidying and reorganizing. There are a few improvements that could be made. Rimoun 1
Danp2 Posted June 21, 2017 Posted June 21, 2017 @benners Very true. However, I believe the crux of the issue is that he's closing Excel and then attempting to perform additional actions in Excel. Rimoun 1 Latest Webdriver UDF Release Webdriver Wiki FAQs
benners Posted June 21, 2017 Posted June 21, 2017 (edited) I agree. I would expect the button to show the "Value Does Not Exist" message on the attempts due to unchecked errors from the _Excel_RangeFind and\or _ArrayExtract functions. @Rimoun Try Danp2's suggestion and comment out the line below just as a test. If the button works multiple times then that's the issue and it needs to be moved somewhere else _Excel_Close($oExcel, Default, True)  Edited June 21, 2017 by benners Rimoun 1
Rimoun Posted June 21, 2017 Author Posted June 21, 2017 19 minutes ago, benners said: I agree. I would expect the button to show the "Value Does Not Exist" message on the attempts due to unchecked errors from the _Excel_RangeFind and\or _ArrayExtract functions. @Rimoun Try Danp2's suggestion and comment out the line below just as a test. If the button works multiple times then that's the issue and it needs to be moved somewhere else _Excel_Close($oExcel, Default, True)  Thanks Guys for help, that solved my issue Sorry for the messy way the I wrote the code with it. attached the sample file that I used with the script. I appreciate your valuable assistance. Thanks info.xlsx
benners Posted June 21, 2017 Posted June 21, 2017 No need to apologize, most of us have been there. When I see others code, it sometimes feels like I still am there. Now I can test my changes, I'll have a look and post back.
benners Posted June 21, 2017 Posted June 21, 2017 Well, after the initial loading, when I selected 1001 from the list and pressed Get Info button, I got an error from the excel udf. As I understand it, the basic operation is to read the information from the sheet, load the Staff# into the listbox, when the Staff# is selected, pressing Get Info should get the related info, Name. Dept, Joining and Id and input them into the inputs. Is this correct? The other labels that are created but not visible because the GUI is to small, they are the text of the excel columns, like Id etc?. Rimoun 1
Rimoun Posted June 21, 2017 Author Posted June 21, 2017 37 minutes ago, benners said: Well, after the initial loading, when I selected 1001 from the list and pressed Get Info button, I got an error from the excel udf. As I understand it, the basic operation is to read the information from the sheet, load the Staff# into the listbox, when the Staff# is selected, pressing Get Info should get the related info, Name. Dept, Joining and Id and input them into the inputs. Is this correct? The other labels that are created but not visible because the GUI is to small, they are the text of the excel columns, like Id etc?. Yes everything exactly as u said. the script will work if u replaced the excel udf as the old has a problem, there is a post was talking about that, in case u didnt find i can shRe my amended excel udf, the problem in rangefind function, just
benners Posted June 21, 2017 Posted June 21, 2017 That's Ok, thanks for the info, I'll have a search. It is easier to read all the sheet in one go into an array then use the array from there on, so that's how I'll work it. Rimoun 1
benners Posted June 21, 2017 Posted June 21, 2017 Well try this. I have commented a lot of things but if there's anything you don't know then just post. The AutoIt Wiki is a good place to pick up info and there's a thread on Best Practices here that's worth a read. I picked up a lot from both. I changed the top input for a combo box, I kept your other  inputs for the info but they would be better as labels. I have included one as an example but commented it out. The inputs text can be deleted. It has no effect on the program but a bit more professional  expandcollapse popup#Region #### Includes ################################ #include <Array.au3> #include <GuiComboBox.au3> #include <ComboConstants.au3> #include <Excel.au3> #include <GUIConstantsEx.au3> #include <GuiListBox.au3> #include <MsgBoxConstants.au3> ;~ #include <StaticConstants.au3> ; only needed for label example #include <WindowsConstants.au3> #EndRegion #### Includes ################################ #Region #### Globals ################################# ; these relate to the columns that contain the information, ; they relate to the ARRAY columns layout NOT the EXCEL sheet columns. Enum will ; assign them a value starting at 0 and incrementing by one. If you add or change the column ; layout in the excel sheet, these will need to be altered Global Enum _ $STAFFNUMBER_COL, _ ; 0 $NAME_COL, _ ; 1 $DEPT_COL, _ ; 2 $JOINING_COL, _ ; 3 $ID_COL, _ ; 4 $MAX_COLS ; 5, or 1 above the last column ; these relate to the controls on the GUI. Only the ones ; we need to update that would be Global are in here ; ### THESE MUST HAVE THE SAME VALUE AS THEIR COLUMN COUNTERPARTS ### Global Enum _ $STAFFNUMBER_CBO, _ ; 0 $NAME_INP, _ ; 1 $DEPT_INP, _ ; 2 $JOINING_INP, _ ; 3 $ID_INP, _ ; 4 $MAX_IDS ; 5, or 1 above the last control ; create an array to hold the id's for the controls ; scope is global so it can be accessed by all. We ; loop through this array later when updating the inputs ; with the array values Global $g_aiControlIDs[$MAX_IDS] ; declare a variable to hold the RangeRead return ; scope is global so it can be accessed by all Global $g_asStaffInfo = '' #EndRegion #### Globals ################################# ; read the full sheet to the array ReadSheetToArray($g_asStaffInfo) ;~ _ArrayDisplay($g_asStaffInfo) ; draw the GUI GUI_Draw() Func GUI_Draw() GUICreate("Rimo System", 270, 500, -1, -1) ; no need to assign the controlID to a variable as ; it's text is constant GUICtrlCreateLabel("Rimo System", 80, 16, 150, 25) GUICtrlSetFont(-1, 14, 800, 0, "MS Serif") ; $STAFFNUMBER_CBO $g_aiControlIDs[$STAFFNUMBER_CBO] = GUICtrlCreateCombo("", 50, 48, 169, 21, BitOR($CBS_DROPDOWNLIST, $CBS_AUTOHSCROLL, $WS_VSCROLL)) GUICtrlSetData(-1, LoadStaffNumberCombo()) ; declare as Local because it's not used outside of this function Local $id_GetInfo_btn = GUICtrlCreateButton("Get Info", 85, 88, 89, 33) ; $NAME_INP - Gets the text from the array and corresponding column GUICtrlCreateLabel($g_asStaffInfo[0][$NAME_COL], 16, 147, 36, 17) $g_aiControlIDs[$NAME_INP] = GUICtrlCreateInput("", 72, 144, 161, 21) ; label example ;~ $g_aiControlIDs[$NAME_INP] = GUICtrlCreateLabel("", 72, 144, 161, 21, $SS_CENTERIMAGE, $WS_EX_STATICEDGE) ;~ GUICtrlSetBkColor(-1, 0xFFFFFF) ; $DEPT_INP - Gets the text from the array and corresponding column GUICtrlCreateLabel($g_asStaffInfo[0][$DEPT_COL], 16, 177, 36, 17) $g_aiControlIDs[$DEPT_INP] = GUICtrlCreateInput("", 72, 176, 161, 21) ; $JOINING_INP - Gets the text from the array and corresponding column GUICtrlCreateLabel($g_asStaffInfo[0][$JOINING_COL], 16, 211, 36, 17) $g_aiControlIDs[$JOINING_INP] = GUICtrlCreateInput("", 72, 208, 161, 21) ; $ID_INP - Gets the text from the array and corresponding column GUICtrlCreateLabel($g_asStaffInfo[0][$ID_COL], 16, 243, 36, 17) $g_aiControlIDs[$ID_INP] = GUICtrlCreateInput("", 72, 240, 161, 21) ; unknown1 - These two will need to be added when you expand the spreadsheet ; you will need to create them in the same way as the others above ;~ GUICtrlCreateLabel("", 16, 272, 36, 17) ;~ Global $id_unknown2_inp = GUICtrlCreateInput("", 72, 272, 161, 21) ; unknown2 ;~ GUICtrlCreateLabel("", 16, 304, 36, 17) ;~ Global $id_unknown2_inp = GUICtrlCreateInput("", 72, 304, 161, 21) ; declare as Local because it's not used outside of this function Local $id_Cancel_btn = GUICtrlCreateButton("Cancel", 75, 416, 121, 25) ; show the gui GUISetState() While 1 Switch GUIGetMsg() Case $GUI_EVENT_CLOSE, $id_Cancel_btn ExitLoop Case $id_GetInfo_btn LoadStaffInfoInputs() ; retreive the info from the array EndSwitch WEnd GUIDelete() EndFunc ;==>GUI_Draw Func LoadStaffNumberCombo() ; set a starting string Local $s_Input = '|' ; loop through the Staff# column of the array For $i = 1 To UBound($g_asStaffInfo) - 1 ; add the values from the rows ($i) $s_Input &= '|' & $g_asStaffInfo[$i][$STAFFNUMBER_COL] Next ; trim the first pipe symbol Return StringTrimLeft($s_Input, 1) EndFunc ;==>LoadStaffNumberCombo Func LoadStaffInfoInputs() ; get the index of the currently selected item in the combo Local $i_ComboIndex = _GUICtrlComboBox_GetCurSel($g_aiControlIDs[$STAFFNUMBER_CBO]) If $i_ComboIndex = -1 Then Return ; nothing selected ; _GUICtrlComboBox_GetCurSel returns a zero based index so ; we need to increase by one to tally with $g_asStaffInfo rows $i_ComboIndex += 1 ; loop through the inputs and add the values. If the number of columns in the excel ; sheet increases and the number of inputs increases, as long as they are added to the global ; enums at the top of the script in the correct order, this should not need adjusting For $i = $NAME_COL To $MAX_COLS - 1 GUICtrlSetData($g_aiControlIDs[$i], $g_asStaffInfo[$i_ComboIndex][$i]) Next EndFunc ;==>LoadStaffInfoInputs Func ReadSheetToArray(ByRef $as_RangeRead) Local $s_ErrorMsg = "Error creating the Excel application object" ; create an instance of excel. Declare as local scope ; as it is only needed in this function Local $o_Excel = _Excel_Open(False) If Not @error Then $s_ErrorMsg = "Error opening workbook" ; open the workbook. Scope as above Local $o_WorkBook = _Excel_BookOpen($o_Excel, 'D:\info.xlsx', False, False) If Not @error Then $s_ErrorMsg = "Error reading excel range" $as_RangeRead = _Excel_RangeRead($o_WorkBook) EndIf EndIf If @error Then Exit MsgBox($MB_SYSTEMMODAL, _ "Excel Error", _ $s_ErrorMsg & $o_WorkBook & @CRLF & _ "@error = " & @error & ", @extended = " & @extended) ; we're are finished with excel now as all the info ; is in the array so it can be closed _Excel_Close($o_Excel) EndFunc ;==>ReadSheetToArray  Rimoun 1
Rimoun Posted June 22, 2017 Author Posted June 22, 2017 12 hours ago, benners said: Well try this. I have commented a lot of things but if there's anything you don't know then just post. The AutoIt Wiki is a good place to pick up info and there's a thread on Best Practices here that's worth a read. I picked up a lot from both. I changed the top input for a combo box, I kept your other  inputs for the info but they would be better as labels. I have included one as an example but commented it out. The inputs text can be deleted. It has no effect on the program but a bit more professional  expandcollapse popup#Region #### Includes ################################ #include <Array.au3> #include <GuiComboBox.au3> #include <ComboConstants.au3> #include <Excel.au3> #include <GUIConstantsEx.au3> #include <GuiListBox.au3> #include <MsgBoxConstants.au3> ;~ #include <StaticConstants.au3> ; only needed for label example #include <WindowsConstants.au3> #EndRegion #### Includes ################################ #Region #### Globals ################################# ; these relate to the columns that contain the information, ; they relate to the ARRAY columns layout NOT the EXCEL sheet columns. Enum will ; assign them a value starting at 0 and incrementing by one. If you add or change the column ; layout in the excel sheet, these will need to be altered Global Enum _ $STAFFNUMBER_COL, _ ; 0 $NAME_COL, _ ; 1 $DEPT_COL, _ ; 2 $JOINING_COL, _ ; 3 $ID_COL, _ ; 4 $MAX_COLS ; 5, or 1 above the last column ; these relate to the controls on the GUI. Only the ones ; we need to update that would be Global are in here ; ### THESE MUST HAVE THE SAME VALUE AS THEIR COLUMN COUNTERPARTS ### Global Enum _ $STAFFNUMBER_CBO, _ ; 0 $NAME_INP, _ ; 1 $DEPT_INP, _ ; 2 $JOINING_INP, _ ; 3 $ID_INP, _ ; 4 $MAX_IDS ; 5, or 1 above the last control ; create an array to hold the id's for the controls ; scope is global so it can be accessed by all. We ; loop through this array later when updating the inputs ; with the array values Global $g_aiControlIDs[$MAX_IDS] ; declare a variable to hold the RangeRead return ; scope is global so it can be accessed by all Global $g_asStaffInfo = '' #EndRegion #### Globals ################################# ; read the full sheet to the array ReadSheetToArray($g_asStaffInfo) ;~ _ArrayDisplay($g_asStaffInfo) ; draw the GUI GUI_Draw() Func GUI_Draw() GUICreate("Rimo System", 270, 500, -1, -1) ; no need to assign the controlID to a variable as ; it's text is constant GUICtrlCreateLabel("Rimo System", 80, 16, 150, 25) GUICtrlSetFont(-1, 14, 800, 0, "MS Serif") ; $STAFFNUMBER_CBO $g_aiControlIDs[$STAFFNUMBER_CBO] = GUICtrlCreateCombo("", 50, 48, 169, 21, BitOR($CBS_DROPDOWNLIST, $CBS_AUTOHSCROLL, $WS_VSCROLL)) GUICtrlSetData(-1, LoadStaffNumberCombo()) ; declare as Local because it's not used outside of this function Local $id_GetInfo_btn = GUICtrlCreateButton("Get Info", 85, 88, 89, 33) ; $NAME_INP - Gets the text from the array and corresponding column GUICtrlCreateLabel($g_asStaffInfo[0][$NAME_COL], 16, 147, 36, 17) $g_aiControlIDs[$NAME_INP] = GUICtrlCreateInput("", 72, 144, 161, 21) ; label example ;~ $g_aiControlIDs[$NAME_INP] = GUICtrlCreateLabel("", 72, 144, 161, 21, $SS_CENTERIMAGE, $WS_EX_STATICEDGE) ;~ GUICtrlSetBkColor(-1, 0xFFFFFF) ; $DEPT_INP - Gets the text from the array and corresponding column GUICtrlCreateLabel($g_asStaffInfo[0][$DEPT_COL], 16, 177, 36, 17) $g_aiControlIDs[$DEPT_INP] = GUICtrlCreateInput("", 72, 176, 161, 21) ; $JOINING_INP - Gets the text from the array and corresponding column GUICtrlCreateLabel($g_asStaffInfo[0][$JOINING_COL], 16, 211, 36, 17) $g_aiControlIDs[$JOINING_INP] = GUICtrlCreateInput("", 72, 208, 161, 21) ; $ID_INP - Gets the text from the array and corresponding column GUICtrlCreateLabel($g_asStaffInfo[0][$ID_COL], 16, 243, 36, 17) $g_aiControlIDs[$ID_INP] = GUICtrlCreateInput("", 72, 240, 161, 21) ; unknown1 - These two will need to be added when you expand the spreadsheet ; you will need to create them in the same way as the others above ;~ GUICtrlCreateLabel("", 16, 272, 36, 17) ;~ Global $id_unknown2_inp = GUICtrlCreateInput("", 72, 272, 161, 21) ; unknown2 ;~ GUICtrlCreateLabel("", 16, 304, 36, 17) ;~ Global $id_unknown2_inp = GUICtrlCreateInput("", 72, 304, 161, 21) ; declare as Local because it's not used outside of this function Local $id_Cancel_btn = GUICtrlCreateButton("Cancel", 75, 416, 121, 25) ; show the gui GUISetState() While 1 Switch GUIGetMsg() Case $GUI_EVENT_CLOSE, $id_Cancel_btn ExitLoop Case $id_GetInfo_btn LoadStaffInfoInputs() ; retreive the info from the array EndSwitch WEnd GUIDelete() EndFunc ;==>GUI_Draw Func LoadStaffNumberCombo() ; set a starting string Local $s_Input = '|' ; loop through the Staff# column of the array For $i = 1 To UBound($g_asStaffInfo) - 1 ; add the values from the rows ($i) $s_Input &= '|' & $g_asStaffInfo[$i][$STAFFNUMBER_COL] Next ; trim the first pipe symbol Return StringTrimLeft($s_Input, 1) EndFunc ;==>LoadStaffNumberCombo Func LoadStaffInfoInputs() ; get the index of the currently selected item in the combo Local $i_ComboIndex = _GUICtrlComboBox_GetCurSel($g_aiControlIDs[$STAFFNUMBER_CBO]) If $i_ComboIndex = -1 Then Return ; nothing selected ; _GUICtrlComboBox_GetCurSel returns a zero based index so ; we need to increase by one to tally with $g_asStaffInfo rows $i_ComboIndex += 1 ; loop through the inputs and add the values. If the number of columns in the excel ; sheet increases and the number of inputs increases, as long as they are added to the global ; enums at the top of the script in the correct order, this should not need adjusting For $i = $NAME_COL To $MAX_COLS - 1 GUICtrlSetData($g_aiControlIDs[$i], $g_asStaffInfo[$i_ComboIndex][$i]) Next EndFunc ;==>LoadStaffInfoInputs Func ReadSheetToArray(ByRef $as_RangeRead) Local $s_ErrorMsg = "Error creating the Excel application object" ; create an instance of excel. Declare as local scope ; as it is only needed in this function Local $o_Excel = _Excel_Open(False) If Not @error Then $s_ErrorMsg = "Error opening workbook" ; open the workbook. Scope as above Local $o_WorkBook = _Excel_BookOpen($o_Excel, 'D:\info.xlsx', False, False) If Not @error Then $s_ErrorMsg = "Error reading excel range" $as_RangeRead = _Excel_RangeRead($o_WorkBook) EndIf EndIf If @error Then Exit MsgBox($MB_SYSTEMMODAL, _ "Excel Error", _ $s_ErrorMsg & $o_WorkBook & @CRLF & _ "@error = " & @error & ", @extended = " & @extended) ; we're are finished with excel now as all the info ; is in the array so it can be closed _Excel_Close($o_Excel) EndFunc ;==>ReadSheetToArray  This sounds very advanced for me. You used only just 10% from the size of my code to do the same. I will study it very well once I am at home, and I am pretty sure that this code will have a great developmental impact on me. Thanks for your great help to rewrite my code in a readable & easy way. Thanks a million.
benners Posted June 22, 2017 Posted June 22, 2017 It only looks that way at first . A lot of your code seemed to be to make the input box act like a combo box with accelerator keys to control it.  I replaced it with a combo box, you can scroll through it with the mouse or arrow keys and it expands to show a list. I hope it functions the way you want compared with the first input box. Everyone has there own preferences on layout, so as long as your code is easily readable, is easy to maintain (even after it's not fresh in your mind) and understandable (more likely to get help here) your half way there. If you do add extra inputs like the unknown input parts, you can add them to the controls array. The declarations below are only there to show you the scope each control needs to be declared in. Don't declare the Globals inside the function, If that makes sense Good luck ;~ GUICtrlCreateLabel("", 16, 272, 36, 17) ;~ Global $id_unknown1_inp = GUICtrlCreateInput("", 72, 272, 161, 21) ; unknown2 ;~ GUICtrlCreateLabel("", 16, 304, 36, 17) ;~ Global $id_unknown2_inp = GUICtrlCreateInput("", 72, 304, 161, 21)  Rimoun 1
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