Jump to content

Sky Slate Blueberry Blackcurrant Watermelon Strawberry Orange Banana Apple Emerald Chocolate
Photo

MyDrfrag script - comments please


  • Please log in to reply
7 replies to this topic
Oxalid
  • Members
  • 5 posts
  • Last active: Sep 01 2009 04:24 PM
  • Joined: 07 Dec 2005
Howdy everyone. I'm new to scripting and have my first script at a point where I'm ready to release for comments.

I always wanted a way to defrag my drives and have my computer shut down when finished.
That way I can do more important things while my computer is defraging (like sleeping). :wink:

You can download the script here: MyDefrag

Posted Image

This script uses the WinXP command line defrag.exe, so this won't work with older versions (not sure about Win2000).
Also, the script uses the cb.exe utility to retrieve output from the command line, so if you don't have this program you
can get it here: cb.zip

Again, this is my first real attempt at a useful script, so I'm very interested in any constructive feedback.
I know I went overboard with the comments in the script, but that was more for my sake in learning the language,
rather then for the pros who browse these forums. :wink:

Thanks in advance to anyone who comments on it, and thanks to everyone I took ideas from in the forums.

toralf
  • Moderators
  • 4035 posts
  • Last active: Aug 20 2014 04:23 PM
  • Joined: 31 Jan 2005
Dear Oxalid,

I'm very impressed:
- The GUI is exellent
- the documentation in the code is fabulous

Very well done.

Here are my remarks:
If you would rewrite this
If SaveWinPos
{
	WinGetPos, Xpos, Ypos,,, %MainTitle%
	IniWrite, %Xpos%, %A_ScriptFullPath%, Settings, Xpos	
	IniWrite, %Ypos%, %A_ScriptFullPath%, Settings, Ypos
}
to this
If SaveWinPos
{
	WinGetPos, Xpos, Ypos,,, %MainTitle%
	IniWrite, x%Xpos% y%Ypos%, %A_ScriptFullPath%, Settings, GuiPos	
}
Else
{
               IniDelete, %A_ScriptFullPath%, Settings , GuiPos
}
Then you can write for this line
IniRead, Xpos, %A_ScriptFullPath%, Settings, Xpos , 327
IniRead, Ypos, %A_ScriptFullPath%, Settings, Ypos , 369
this
IniRead, Xpos, %A_ScriptFullPath%, Settings, GuiPos, %A_Space%
and for this
If SaveWinPos
	Gui, Show, x%Xpos% y%Ypos%, %MainTitle%
Else
	Gui, Show, Center, %MainTitle%
this
Gui, Show, %GuiPos%, %MainTitle%
Center will be automatic if GuiPos is space

I would suggest to give the buttons of your GUI a v- and g-label. It helps to reas and search in the code and you could use doublicate labels but with different actions (even that it is not needed in this case). And you can make some buttons inactive at start up, e.g. the Defrag button, and activate him when the user has selected a drive.


You could rewrite this
If AnalyzeAfterDefrag
	{
		Menu, %A_ThisMenu%, Uncheck, %A_ThisMenuItem%
		AnalyzeAfterDefrag := 0
	}
	Else
	{
		Menu, %A_ThisMenu%, Check, %A_ThisMenuItem%
		AnalyzeAfterDefrag := 1
	}
to this
If AnalyzeAfterDefrag
		Menu, %A_ThisMenu%, Uncheck, %A_ThisMenuItem%
	Else
		Menu, %A_ThisMenu%, Check, %A_ThisMenuItem%
	AnalyzeAfterDefrag := not AnalyzeAfterDefrag
etc for the other if statements


I would recommend to use a separate ini file instead of the code file itself. Then this script can be compiled to an exe. and you could FileInstall the cb.exe as well.


As a nice addition I would suggest to look for an icon form the shell32.dll and use that as an icon for your app.

That's it for now. A very nice script.
Ciao
toralf
 
I use the latest AHK version (1.1.15+)
Please ask questions in forum on ahkscript.org. Why?
For online reference please use these Docs.

jballi
  • Members
  • 1029 posts
  • Last active:
  • Joined: 01 Oct 2005
1st of all, good job! A program that I will consider although I really like Sysinternal's contig program because you can defrag all drives at the same time.

2nd, everything toralf said, especially the part about pulling the ini out of the program. Here's the code I use to set the ini file name:
StringLeft $ScriptName,A_ScriptName,instr(A_ScriptName,".",N,0)-1
$INIFile=%A_ScriptDir%\%$ScriptName%.ini

Finally, the program is incorrectly identifying virtual drives (example: drives created via the DOS subst command) to defrag. I'm not sure how to exclude them but the degrag program does issue a "The volume identifier is not valid." message when you run it (analyze and defrag).

Them be my thoughts...

toralf
  • Moderators
  • 4035 posts
  • Last active: Aug 20 2014 04:23 PM
  • Joined: 31 Jan 2005

Here's the code I use to set the ini file name:

StringLeft $ScriptName,A_ScriptName,instr(A_ScriptName,".",N,0)-1
$INIFile=%A_ScriptDir%\%$ScriptName%.ini

Dear jballi,
I would recommend to use SplitPath, since the file name might have several dots in its name plus the one for the extention.

I generally just add a ".ini" to A_ScriptName. Then it may be myscript.ahk.ini or myscript.exe.ini
Ciao
toralf
 
I use the latest AHK version (1.1.15+)
Please ask questions in forum on ahkscript.org. Why?
For online reference please use these Docs.

jballi
  • Members
  • 1029 posts
  • Last active:
  • Joined: 01 Oct 2005

I would recommend to use SplitPath, since the file name might have several dots in its name plus the one for the extention.


You are correct, oh wise one. Since A_ScriptName contains a complete path, SplitPath works just as good in this case. i.e.
SplitPath, A_ScriptName,,,,$ScriptName
$INIFile=%A_ScriptDir%\%$ScriptName%.ini
Since the instr is doing a reverse search for the "." character, it will return the same value in this case. For some reason, I didn't think to use the SplitPath command.

Personally, I like to use the ScriptNameNoExt.INI format when defining a configuration file because it is a bit more standardized (what the heck is standard?) and it allows me to use the same INI file regardless if the script is compiled or not.

Them be my thoughts.

quatermass
  • Members
  • 220 posts
  • Last active: Dec 16 2013 09:00 PM
  • Joined: 14 Dec 2005
Very nice. But it doesn't do anything with Windows 2000....
I guess the defrag commandline options may be different.
Stuart Halliday

shekel2000
  • Members
  • 3 posts
  • Last active: Apr 21 2006 08:50 PM
  • Joined: 06 Aug 2005
Now only if you add scheduling options this would be perfect...

PhiLho
  • Moderators
  • 6850 posts
  • Last active: Jan 02 2012 10:09 PM
  • Joined: 27 Dec 2005
NT-like Windows have Scheduled Tasks in the Control Panel...
Plus you have lot of freewares able to schedule some task.
Posted Image vPhiLho := RegExReplace("Philippe Lhoste", "^(\w{3})\w*\s+\b(\w{3})\w*$", "$1$2")