|
|
Posted by Highlander on 11/19/32 11:33
Al Dunbar wrote:
> "Highlander" <tron9901@msn.com> wrote in message
> news:1133322022.454485.96000@f14g2000cwb.googlegroups.com...
> > Al Dunbar wrote:
> >> "Highlander" <tron9901@msn.com> wrote in message
> >> news:1133209896.037785.269910@o13g2000cwo.googlegroups.com...
>
> <snip>
>
> >> > Sub Window_Onload
> >> > self.Focus()
> >> > self.ResizeTo 600,500
> >> > self.MoveTo 200,50
> >> >
> >> > ' Clear all variables in order for the
> >> > ' "Enable Change Database button" IF statement to work
> >>
> >> Would not be as much of an issue if you avoided using global variables.
> >
> > Since the "Enable Change Database button" IF statement has to be in all
> > 3 subroutines (ServerButton, ReleaseSelectList and DBSelectList), I had
> > to make the 3 variables (strServer, strRelease and strDatabase) global.
>
> As you wrote it, perhaps, however I try to avoid global variables at almost
> all costs.
>
That's my point - is there another way to write it? You mention below
that there may be other ways, i.e. put that code in a small subroutine
and call it from wherever you need to.
> > Done.
> > Extracting the common code from the true and false blocks worked great.
> > I used this same logic in other areas of the script and overall I
> > trimmed between 25 and 30 lines. Thanks Al!
>
> The issue is not trimming lines; the issue is the simplicity and
> maintainability of the code.
>
That makes sense. I would imagine though that trimming the code and
removing redundancy where you can is part of making it simpler and
easier to maintain.
Ideally a script would be written "right" in the first place, the right
logic, right commands, right format, etc. Simplicity and
maintainability would be there from the outset. In my case I don't have
the experience and never learned VBScript from the ground up, the
basics, the guidelines, etc. I create scripts in a crude way, by
bolting together code snippets from many different sources, and then
attempting to pare it down so that it has some semblance of efficiency,
simplicity and maintainability. That's why I value your opinion (as
well as other helpful responders on Usenet) highly. You help me get
there.
> > Any suggestions on my remaining questions?
>
> I didn't answer them all before because you posted a script of nearly 400
> lines. As it is, and without doing an exhaustive (and exhausting) analysis
> of the whole thing (which would require a lot of deductive reasoning to
> figure out), all I can do is make a few suggestions based on what I see in
> your code (see a few more way below...). Unfortunately, it might be that a
> better next version would actually do things differently.
>
> > - The conditional IF statement used to enable the "Change Database"
> > button...I've had to put it in multiple subroutines. Is there a better
> > way?
>
> You could put that code in a small subroutine and call it from wherever you
> need to. This would localize your references to these global variables.
>
Can you give me an example of what this small subroutine would look
like, and how I would call it?
> > - I'm using the sysinternals.com utility PSEXEC to remotely change the
> > Registry. Is there a better way?
>
> There are other ways. What is it that you might think needs improvement in
> this aspect of your script?
>
> > - I'm using the sysinternals.com utility PSEXEC to remotely restart
> > IIS. Is there a better way?
>
> There may be other ways. What is it that you might think needs improvement
> in this aspect of your script?
>
I just wondered if there is functionality within VBScript syntax that
will perform the same tasks. Although shelling out and using PSEXEC
works fine in this script, I just considered that using inherent
VBScript capability (if it exists) is "cleaner" than shelling out and
using 3rd party utilities.
> > - I'm using repeated entries of the HTML " " to insert a large
> > number of spaces in between text and listboxes. Is there a better way?
>
> Not really a vbscript question, but it depends on why you need whitespace.
> To be honest, I am not going to run your HTA to see what it looks like, but
> you might be able to produce a better laid out window using HTML tables.
>
> Here are some more suggestions for you...
>
> - Use a more typical indentation style. For example, the "sub" and "end sub"
> statements should be indented less than what they enclose.
>
> - this code:
>
> ' If the first bit of the attributes byte is set to 1,
> ' the file is Read Only.
> ' If this is the case, clear the first bit.
>
> If f.attributes and 1 Then
> f.attributes = f.attributes - 1
> End If
>
> would be more correctly coded as:
>
> f.attributes = f.attributes or -2
>
I tried "f.attributes = f.attributes or -2" and it didn't clear the
Read Only bit; it actually gave the file hidden and system attributes.
> - in your "cleanup section" at the end, you seem to set all variables to
> nothing. This is not required of any variable that does not contain an
> object. And of those variables containing objects, not all need be dismissed
> this way.
>
Since I've seen the "set variable = nothing" in many scripts, and have
been told that this is done to release memory, I assumed that it needed
to be done with every variable. If this isn't the case, what's the
criteria for when it should be used for a given variable?
> /Al
Thanks for your time Al.
- Dave
Navigation:
[Reply to this message]
|