rohit Posted December 29, 2007 Share Posted December 29, 2007 This is my first useful script expandcollapse popup#include-once #include<array.au3> Func _installinfo() Global $arr[1],$list[1],$k=0,$i = 1, $unin = "HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall" Do $arr[$i - 1] = RegEnumKey($unin, $i) $i += 1 ReDim $arr[$i] Until StringLeft($arr[$i - 2], 12) = "no more data" _ArrayDelete($arr, $i - 1) _ArrayDelete($arr, $i - 2) $n = UBound($arr) ;Get the uninstall keys & Display names Local $keys[$n],$_keys[1] For $i = 1 To $n $keys[$i - 1] = RegRead($unin & "\" & $arr[$i - 1], "UninstallString") $y = RegRead($unin & "\" & $arr[$i - 1], "DisplayName") $arr[$i - 1] = $y If ($arr[$i-1]="") Or ($keys[$i-1]="") Then $keys[$i-1]="" $arr[$i-1]="" EndIf ;remove invalid If Not $arr[$i-1] = "" Then $list[$k]=$arr[$i-1] $_keys[$k] = $keys[$i-1] $k+=1 ReDim $list[$k+2] ReDim $_keys[$k+2] EndIf ;resize the info Next For $i = $k to $k+1 _ArrayDelete($list,$i) _ArrayDelete($_keys,$i) Next $n = UBound($list) Local $reg[$n][2] For $i = 1 to $n $reg[$i-1][0] = $list[$i-1] $reg[$i-1][1] = $_keys[$i-1] Next Return $reg EndFunc Please give suggestions on improvement.... Link to comment Share on other sites More sharing options...
JerryD Posted December 29, 2007 Share Posted December 29, 2007 This is my first useful script Please give suggestions on improvement.... Your script certainly works, and it's great to see you're grasping how to use AutoIt excellently. That said, please don't take offense, but the script is very inefficient, and doesn't adhere to some unspoken standards, which if you're new to AutoIt is certainly understandable. Specifically, AutoIt functions that return an array typically have the number of elements stored in the [0] or [0][0] element of the returned array - like IniReadSection(). And you're doing a LOT of work deleting array elements that are better not being created in the first place. Here's how I would write that functionFunc _UninstallInfo() Local $StartTime = TimerInit() Const $sUninstallKey = 'HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall' Local $aRetAry[1][2], $i = 1, $sCurKey, $sUninstallString, $sDisplayName $aRetAry[0][0] = 0 Do $sCurKey = RegEnumKey ( $sUninstallKey, $i ) If @error Then ExitLoop $sUninstallString = RegRead ( $sUninstallKey & '\' & $sCurKey, 'UninstallString' ) $sDisplayName = RegRead ( $sUninstallKey & '\' & $sCurKey, 'DisplayName' ) If $sUninstallString<>'' AND $sDisplayName<>'' Then $aRetAry[0][0] += 1 ReDim $aRetAry[$aRetAry[0][0]+1][3] $aRetAry[$aRetAry[0][0]][0] = $sDisplayName $aRetAry[$aRetAry[0][0]][1] = $sUninstallString EndIf $i += 1 Until 1=2 MsgBox ( 0, 'Dif', TimerDiff ( $StartTime ) ) Return $aRetAry EndFuncNote that the TimerInit and MsgBox calls were just to check out how much time this took versus your script. Typically your function took about twice as long as mine (uncompiled). While scripts are typically small and inefficiencies don't eat up much time, it eventually adds up! Anyway, you asked for some feedback, so there you have it. As I said initially there's nothing wrong with the script, and I hope you don't take it the wrong way! Link to comment Share on other sites More sharing options...
rohit Posted December 29, 2007 Author Share Posted December 29, 2007 Wow ...that was good .... thanx...I was a little hesitant to use a 2D array... Link to comment Share on other sites More sharing options...
vash Posted December 30, 2007 Share Posted December 30, 2007 Even faster: Func _UninstallInfo() Local Const $sUninstallKey='HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall',$increment=256 Local $size=$increment,$aRetAry[$size][2],$i=1,$sCurKey,$sUninstallString,$sDisplayName $aRetAry[0][0]=0 $sCurKey=RegEnumKey($sUninstallKey,$i) While Not @error $sUninstallString = RegRead ( $sUninstallKey & '\' & $sCurKey, 'UninstallString' ) $sDisplayName = RegRead ( $sUninstallKey & '\' & $sCurKey, 'DisplayName' ) If $sUninstallString<>'' And $sDisplayName<>'' Then $aRetAry[0][0]+=1 If $aRetAry[0][0]==$size Then $size+=$increment ReDim $aRetAry[$size][2] EndIf $aRetAry[$aRetAry[0][0]][0] = $sDisplayName $aRetAry[$aRetAry[0][0]][1] = $sUninstallString EndIf $i+=1 $sCurKey=RegEnumKey($sUninstallKey,$i) WEnd ReDim $aRetAry[$aRetAry[0][0]][2] Return $aRetAry EndFunc Benchmark (100 iterations): mine:4754ms JerryD's:14191ms It's actually just an optimization of JerryD's code. It's faster because it doesn't resizes the array nearly as much. Resizing an array is slow because it means creating a new array, copying the old array into it and throwing the old array away. Link to comment Share on other sites More sharing options...
vash Posted December 30, 2007 Share Posted December 30, 2007 Oops, on ReDim $aRetAry[$aRetAry[0][0]][2] it should be ReDim $aRetAry[$aRetAry[0][0]+1][2]. Link to comment Share on other sites More sharing options...
JerryD Posted December 30, 2007 Share Posted December 30, 2007 Very nice! I tried reworking the code to do two passes through the RegEnumKey loop - one to get how many entries there would be and one to assign them, but your technique of adjusting the array in chunks is still faster. Sweet! Link to comment Share on other sites More sharing options...
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