Amateur would like code critique

[MS]

I'm strictly an amteur. Self taught via the help files.

Here is an example of a DB I've done. It is quite simple to some of the ones
I do for my workplace, but it is an example of the way i work. I have
received enormous help from this NG over the years, and I was wondering if
you experts could have a look and see how I'm doing, and if I'm doing
anything blatantly silly.

This app is to quickly calculate the cost of transport around Australia by a
particular courier company from a particular deaparture point.

Thanks in advance.

Cheers to all!
http://members.ozemail.com.au/~tma1/...terExpress.zip

[Jack MacDonald]

Looks like it does the job that it was intended to do, and that should
be a major evaluation criteria. Keep up the good work!

There are a couple things you could do differently:

1. Get in the habit of:
Dim rst as DAO.Recordset
instead of
Dim rst as Recordset

If you ever work on a system with both DAO and ADO, you will find that
removing the ambiguity is vital.

2. Why did you put your Calculate procedure in a separate module? It
could easily work within the form's code module. Doing so would
simplify some of your code -- easier to refer to various controls from
within the form's code module.

3. You have several procedures with exactly the same functionality
e.g. TxtPC_GotFocus, TxtPC_Click, and TxtHt_Click. Once I see a
repetetive code section, I try to place it into a single procedure
that is called from several locations, using parameters.
HTH
On Thu, 07 Jul 2005 03:01:55 GMT, "MS" <Em***@Myemail.com> wrote:

I'm strictly an amteur. Self taught via the help files.

Here is an example of a DB I've done. It is quite simple to some of the ones
I do for my workplace, but it is an example of the way i work. I have
received enormous help from this NG over the years, and I was wondering if
you experts could have a look and see how I'm doing, and if I'm doing
anything blatantly silly.

This app is to quickly calculate the cost of transport around Australia by a
particular courier company from a particular deaparture point.

Thanks in advance.

Cheers to all!
http://members.ozemail.com.au/~tma1/...terExpress.zip

**********************
ja**************@telusTELUS.net
remove uppercase letters for true email
http://www.geocities.com/jacksonmacd/ for info on MS Access security

[MS]

"Jack MacDonald" <ja**************@telus.net> wrote in message
news:uq********************************@4ax.com...

Looks like it does the job that it was intended to do, and that should
be a major evaluation criteria. Keep up the good work!

There are a couple things you could do differently:

1. Get in the habit of:
Dim rst as DAO.Recordset
instead of
Dim rst as Recordset

If you ever work on a system with both DAO and ADO, you will find that
removing the ambiguity is vital.

2. Why did you put your Calculate procedure in a separate module? It
could easily work within the form's code module. Doing so would
simplify some of your code -- easier to refer to various controls from
within the form's code module.

3. You have several procedures with exactly the same functionality
e.g. TxtPC_GotFocus, TxtPC_Click, and TxtHt_Click. Once I see a
repetetive code section, I try to place it into a single procedure
that is called from several locations, using parameters.

Thanks for taking the time to have a look!

I'll take on board the suggestions. I often tend to "duplicate" some things
like "Click" and "GotFocus" as I'm not sure that clicking on a control and
tabbing onto it are the same.

As far as the Calculate module being in it's own module, this procedure is
called both when the Calculate button is pushed and when the Destination
Combo is triggered, The reason for this that I wanted the calculation to be
effected after changing the Destination, but if the weight criteria is
changed, I wanted the user to be able to "recalculate" with the new weight.

Calling the procedure from a module meant that if I wanted to change the
formula, I'd only have to do it once.

I'll investigate the differences between DAO and ADO.

Thanks! All the best!

[Jack MacDonald]

On Fri, 08 Jul 2005 04:30:06 GMT, "MS" <Em***@Myemail.com> wrote:

"Jack MacDonald" <ja**************@telus.net> wrote in message
news:uq********************************@4ax.com.. .

Looks like it does the job that it was intended to do, and that should
be a major evaluation criteria. Keep up the good work!

There are a couple things you could do differently:

1. Get in the habit of:
Dim rst as DAO.Recordset
instead of
Dim rst as Recordset

If you ever work on a system with both DAO and ADO, you will find that
removing the ambiguity is vital.

2. Why did you put your Calculate procedure in a separate module? It
could easily work within the form's code module. Doing so would
simplify some of your code -- easier to refer to various controls from
within the form's code module.

3. You have several procedures with exactly the same functionality
e.g. TxtPC_GotFocus, TxtPC_Click, and TxtHt_Click. Once I see a
repetetive code section, I try to place it into a single procedure
that is called from several locations, using parameters.

Thanks for taking the time to have a look!

I'll take on board the suggestions. I often tend to "duplicate" some things
like "Click" and "GotFocus" as I'm not sure that clicking on a control and
tabbing onto it are the same.

Nothing wrong with that strategy, and it goes towards my point. You
make a *common* procedure in your form's class module, and call that
procedure from *both* events. That way, you don't duplicate the code.


As far as the Calculate module being in it's own module, this procedure is
called both when the Calculate button is pushed and when the Destination
Combo is triggered, The reason for this that I wanted the calculation to be
effected after changing the Destination, but if the weight criteria is
changed, I wanted the user to be able to "recalculate" with the new weight.

Calling the procedure from a module meant that if I wanted to change the
formula, I'd only have to do it once.
My point exactly... Perhaps you are unaware that you can put
procedures in the form's code in addition to the event procedures for
the controls??? Something like this
Sub txtButton_click
doStuff
end sub

sub cboWhatever_afterupdate
doStuff
end sub

sub doStuff()
... a bunch of commands
end sub
I would put the procedure into a separate module *only* if it was
required to be called from two different forms.


I'll investigate the differences between DAO and ADO.

Thanks! All the best!

**********************
ja**************@telusTELUS.net
remove uppercase letters for true email
http://www.geocities.com/jacksonmacd/ for info on MS Access security

[MS]

"Jack MacDonald" <ja**************@telus.net> wrote in message
news:tq********************************@4ax.com...

On Fri, 08 Jul 2005 04:30:06 GMT, "MS" <Em***@Myemail.com> wrote:

"Jack MacDonald" <ja**************@telus.net> wrote in message
news:uq********************************@4ax.com. ..

Looks like it does the job that it was intended to do, and that should
be a major evaluation criteria. Keep up the good work!

There are a couple things you could do differently:

1. Get in the habit of:
Dim rst as DAO.Recordset
instead of
Dim rst as Recordset

If you ever work on a system with both DAO and ADO, you will find that
removing the ambiguity is vital.

2. Why did you put your Calculate procedure in a separate module? It
could easily work within the form's code module. Doing so would
simplify some of your code -- easier to refer to various controls from
within the form's code module.

3. You have several procedures with exactly the same functionality
e.g. TxtPC_GotFocus, TxtPC_Click, and TxtHt_Click. Once I see a
repetetive code section, I try to place it into a single procedure
that is called from several locations, using parameters.

Thanks for taking the time to have a look!

I'll take on board the suggestions. I often tend to "duplicate" some
things
like "Click" and "GotFocus" as I'm not sure that clicking on a control and
tabbing onto it are the same.

Nothing wrong with that strategy, and it goes towards my point. You
make a *common* procedure in your form's class module, and call that
procedure from *both* events. That way, you don't duplicate the code.

As far as the Calculate module being in it's own module, this procedure is
called both when the Calculate button is pushed and when the Destination
Combo is triggered, The reason for this that I wanted the calculation to
be
effected after changing the Destination, but if the weight criteria is
changed, I wanted the user to be able to "recalculate" with the new
weight.

Calling the procedure from a module meant that if I wanted to change the
formula, I'd only have to do it once.

My point exactly... Perhaps you are unaware that you can put
procedures in the form's code in addition to the event procedures for
the controls??? Something like this
Sub txtButton_click
doStuff
end sub

sub cboWhatever_afterupdate
doStuff
end sub

sub doStuff()
... a bunch of commands
end sub
I would put the procedure into a separate module *only* if it was
required to be called from two different forms.

Of course! That's the way I've done it in the past - is more logical! Thanks