By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
448,712 Members | 1,591 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 448,712 IT Pros & Developers. It's quick & easy.

vb.net class

P: n/a

I have written my first Class and am posting it to this newsgroup with hopes
that I can get some feedback on what is right and what is wrong with it.
All comments are welcome, but more interested in what would make it better.
If I have totally missed on what a class is used for please inform me.

Thanks,

Thomas
'************************************************* *************************************
'************************************************* *************************************
'**** This class accepts one parameter. This parameter is passed as a
Date/Time ****
'**** array. From this variable the Class calculates and exposes three
****
'**** properties. AvgTime - which is average time between each of the
values ****
'**** in the array. TimeSpan - which is the length of time between the
first ****
'**** and last date/time in the array. Properties are returned as Minute
and ****
'**** Second values only. Count - which is the number of usable Date/Time
groups ****
'**** in the passed array
****
'************************************************* *************************************
'************************************************* *************************************

Public Class clsTime

Private _AvgTime As String
Private _TimeSpan As String
Private _Count As Integer
Private datFirst As Date
Private datLast As Date
Private intFirst As Integer = 0
Private intMinute As Integer = 0
Private intSecond As Integer = 0

#Region "Constructors"

Public Sub New(ByVal aryTime() As String)

_AvgTime = funAvgTime(aryTime)
_TimeSpan = funTimeSpan(aryTime)
_Count = intFirst

End Sub

#End Region

#Region "Properties"

Public Property AvgTime() As String
Get
Return _AvgTime
End Get
Set(ByVal value As String)
_AvgTime = value
End Set
End Property

Public Property TimeSpan() As String
Get
Return _TimeSpan
End Get
Set(ByVal value As String)
_TimeSpan = value
End Set
End Property

Public Property Count() As Integer
Get
Return _Count
End Get
Set(ByVal value As Integer)
_Count = value
End Set
End Property

#End Region

#Region "Functions"

Private Function funAvgTime(ByVal aryTime() As String) As String

Dim intAvTime As Integer = 0
intFirst = 0
intMinute = 0
intSecond = 0

Array.Sort(aryTime)

For x As Integer = 0 To aryTime.GetUpperBound(0)
If IsDate(aryTime(x)) Then
intFirst += 1
If intFirst = 1 Then
datFirst = CDate(aryTime(x))
End If
datLast = CDate(aryTime(x))

If intFirst > 1 Then
intAvTime += DateDiff(DateInterval.Second,
CDate(aryTime(x - 1)), CDate(aryTime(x)))
intAvTime = System.Math.Abs(intAvTime)
End If
End If
Next

If intFirst > 0 Then
intAvTime = intAvTime / intFirst
If intAvTime > 60 Then
intMinute = intAvTime \ 60
intSecond = intAvTime Mod 60
Return CStr(intMinute) & ":" & CStr(intSecond)
Else
Return CStr(intAvTime) & " seconds"
End If
Else
Return String.Empty
End If

End Function

Private Function funTimeSpan(ByVal aryTimeSpan() As String) As String

Dim intLength As Integer = 0
intMinute = 0
intSecond = 0

If IsDate(datLast) AndAlso IsDate(datFirst) Then
intLength = DateDiff(DateInterval.Second, datFirst, datLast)
If intLength > 60 Then
intMinute = intLength \ 60
intSecond = intLength Mod 60
Return CStr(intMinute) & ":" & CStr(intSecond)
Else
Return CStr(intLength) & " seconds"
End If
Else
Return String.Empty
End If
End Function

#End Region

End Class

--
Posted via NewsDemon.com - Premium Uncensored Newsgroup Service
------->>>>>>http://www.NewsDemon.com<<<<<<------
Unlimited Access, Anonymous Accounts, Uncensored Broadband Access
Nov 21 '05 #1
Share this Question
Share on Google+
13 Replies


P: n/a
I would declare the following variables inside the functions that use them:

Private datFirst As Date
Private datLast As Date
Private intFirst As Integer = 0
Private intMinute As Integer = 0
Private intSecond As Integer = 0

There is no need for them to be declared at the Class level.

Venki
"th*****@msala.net" wrote:

I have written my first Class and am posting it to this newsgroup with hopes
that I can get some feedback on what is right and what is wrong with it.
All comments are welcome, but more interested in what would make it better.
If I have totally missed on what a class is used for please inform me.

Thanks,

Thomas
'************************************************* *************************************
'************************************************* *************************************
'**** This class accepts one parameter. This parameter is passed as a
Date/Time ****
'**** array. From this variable the Class calculates and exposes three
****
'**** properties. AvgTime - which is average time between each of the
values ****
'**** in the array. TimeSpan - which is the length of time between the
first ****
'**** and last date/time in the array. Properties are returned as Minute
and ****
'**** Second values only. Count - which is the number of usable Date/Time
groups ****
'**** in the passed array
****
'************************************************* *************************************
'************************************************* *************************************

Public Class clsTime

Private _AvgTime As String
Private _TimeSpan As String
Private _Count As Integer
Private datFirst As Date
Private datLast As Date
Private intFirst As Integer = 0
Private intMinute As Integer = 0
Private intSecond As Integer = 0

#Region "Constructors"

Public Sub New(ByVal aryTime() As String)

_AvgTime = funAvgTime(aryTime)
_TimeSpan = funTimeSpan(aryTime)
_Count = intFirst

End Sub

#End Region

#Region "Properties"

Public Property AvgTime() As String
Get
Return _AvgTime
End Get
Set(ByVal value As String)
_AvgTime = value
End Set
End Property

Public Property TimeSpan() As String
Get
Return _TimeSpan
End Get
Set(ByVal value As String)
_TimeSpan = value
End Set
End Property

Public Property Count() As Integer
Get
Return _Count
End Get
Set(ByVal value As Integer)
_Count = value
End Set
End Property

#End Region

#Region "Functions"

Private Function funAvgTime(ByVal aryTime() As String) As String

Dim intAvTime As Integer = 0
intFirst = 0
intMinute = 0
intSecond = 0

Array.Sort(aryTime)

For x As Integer = 0 To aryTime.GetUpperBound(0)
If IsDate(aryTime(x)) Then
intFirst += 1
If intFirst = 1 Then
datFirst = CDate(aryTime(x))
End If
datLast = CDate(aryTime(x))

If intFirst > 1 Then
intAvTime += DateDiff(DateInterval.Second,
CDate(aryTime(x - 1)), CDate(aryTime(x)))
intAvTime = System.Math.Abs(intAvTime)
End If
End If
Next

If intFirst > 0 Then
intAvTime = intAvTime / intFirst
If intAvTime > 60 Then
intMinute = intAvTime \ 60
intSecond = intAvTime Mod 60
Return CStr(intMinute) & ":" & CStr(intSecond)
Else
Return CStr(intAvTime) & " seconds"
End If
Else
Return String.Empty
End If

End Function

Private Function funTimeSpan(ByVal aryTimeSpan() As String) As String

Dim intLength As Integer = 0
intMinute = 0
intSecond = 0

If IsDate(datLast) AndAlso IsDate(datFirst) Then
intLength = DateDiff(DateInterval.Second, datFirst, datLast)
If intLength > 60 Then
intMinute = intLength \ 60
intSecond = intLength Mod 60
Return CStr(intMinute) & ":" & CStr(intSecond)
Else
Return CStr(intLength) & " seconds"
End If
Else
Return String.Empty
End If
End Function

#End Region

End Class

--
Posted via NewsDemon.com - Premium Uncensored Newsgroup Service
------->>>>>>http://www.NewsDemon.com<<<<<<------
Unlimited Access, Anonymous Accounts, Uncensored Broadband Access

Nov 21 '05 #2

P: n/a
Also, realize that all your functions are declared as Private, you cannot
access them from outside the class. Declare them Public so you can invoke
them.

Venki

"th*****@msala.net" wrote:

I have written my first Class and am posting it to this newsgroup with hopes
that I can get some feedback on what is right and what is wrong with it.
All comments are welcome, but more interested in what would make it better.
If I have totally missed on what a class is used for please inform me.

Thanks,

Thomas
'************************************************* *************************************
'************************************************* *************************************
'**** This class accepts one parameter. This parameter is passed as a
Date/Time ****
'**** array. From this variable the Class calculates and exposes three
****
'**** properties. AvgTime - which is average time between each of the
values ****
'**** in the array. TimeSpan - which is the length of time between the
first ****
'**** and last date/time in the array. Properties are returned as Minute
and ****
'**** Second values only. Count - which is the number of usable Date/Time
groups ****
'**** in the passed array
****
'************************************************* *************************************
'************************************************* *************************************

Public Class clsTime

Private _AvgTime As String
Private _TimeSpan As String
Private _Count As Integer
Private datFirst As Date
Private datLast As Date
Private intFirst As Integer = 0
Private intMinute As Integer = 0
Private intSecond As Integer = 0

#Region "Constructors"

Public Sub New(ByVal aryTime() As String)

_AvgTime = funAvgTime(aryTime)
_TimeSpan = funTimeSpan(aryTime)
_Count = intFirst

End Sub

#End Region

#Region "Properties"

Public Property AvgTime() As String
Get
Return _AvgTime
End Get
Set(ByVal value As String)
_AvgTime = value
End Set
End Property

Public Property TimeSpan() As String
Get
Return _TimeSpan
End Get
Set(ByVal value As String)
_TimeSpan = value
End Set
End Property

Public Property Count() As Integer
Get
Return _Count
End Get
Set(ByVal value As Integer)
_Count = value
End Set
End Property

#End Region

#Region "Functions"

Private Function funAvgTime(ByVal aryTime() As String) As String

Dim intAvTime As Integer = 0
intFirst = 0
intMinute = 0
intSecond = 0

Array.Sort(aryTime)

For x As Integer = 0 To aryTime.GetUpperBound(0)
If IsDate(aryTime(x)) Then
intFirst += 1
If intFirst = 1 Then
datFirst = CDate(aryTime(x))
End If
datLast = CDate(aryTime(x))

If intFirst > 1 Then
intAvTime += DateDiff(DateInterval.Second,
CDate(aryTime(x - 1)), CDate(aryTime(x)))
intAvTime = System.Math.Abs(intAvTime)
End If
End If
Next

If intFirst > 0 Then
intAvTime = intAvTime / intFirst
If intAvTime > 60 Then
intMinute = intAvTime \ 60
intSecond = intAvTime Mod 60
Return CStr(intMinute) & ":" & CStr(intSecond)
Else
Return CStr(intAvTime) & " seconds"
End If
Else
Return String.Empty
End If

End Function

Private Function funTimeSpan(ByVal aryTimeSpan() As String) As String

Dim intLength As Integer = 0
intMinute = 0
intSecond = 0

If IsDate(datLast) AndAlso IsDate(datFirst) Then
intLength = DateDiff(DateInterval.Second, datFirst, datLast)
If intLength > 60 Then
intMinute = intLength \ 60
intSecond = intLength Mod 60
Return CStr(intMinute) & ":" & CStr(intSecond)
Else
Return CStr(intLength) & " seconds"
End If
Else
Return String.Empty
End If
End Function

#End Region

End Class

--
Posted via NewsDemon.com - Premium Uncensored Newsgroup Service
------->>>>>>http://www.NewsDemon.com<<<<<<------
Unlimited Access, Anonymous Accounts, Uncensored Broadband Access

Nov 21 '05 #3

P: n/a

1) Naming (minor)
Dubious on the naming clsTime.
- The newer naming standards have moved away from the Hungarian
notation. Pretty much everything is a class now
2) Ctors
- If the class is intended to work upon date/time values then pass
them in as such rather than strings.
- if we are stuck with the values being strings then we are it
would be better if we can avoid this

- If possible, an arraylist is simpler for working with. It will
not impact this class but when generating the inputs an arraylist will
probably make things easier.

- Calculate the properties when needed instead of generating them at
time of construction. You may not always use both
- you will need to either store a reference to the incoming
date/times (if you want the average/span to update if you change any of
the original values)
or, copy the values if you want a snapshot view.

2)
Private _Count As Integer
Private datFirst As Date
Private datLast As Date
Private intFirst As Integer = 0
Private intMinute As Integer = 0
Private intSecond As Integer = 0

do not need to exist at the class level
3) Properies

Count can be generated from the time list, not need to store
It should be read only
AvgTime and TimeSpan should both return TimeSpan

They both should be read-only
If allowing the inputs to be modified then you will want to
recalc them every time (no variables to store them,
Private _AvgTime As String
Private _TimeSpan As String
both disappear) or else you calc them only the first time they
are used, store them in the class and each subsequent time, return the
stored value.
4) Functionality Questions?

What are the pre-reqs for the incoming data?
Are they all guaranteed to be valid times?
if no, what do we do with the invalid times?
if yes, why are you testing IsDate()?
Are they guaranteed to be in time sequence?
if no, then funTimeSpan() will not give the correct value - you
need to find the first and last time and then find the difference
if yes, then why are we using System.Math.Abs(), t+1 - t will
always be non-negative
5) Add a shared function to the class that will create a string from a
TimeSpan that will do the correct formatting for you or you can put in
AsString() versions of the properties.
By providing only String() versions of the output you limit
how/where it can be used. Any class that wants to use the values needs
to be able to parse the string back to a timespan so it needs to know
in what format you have stored it. Which may change.

a) dim tm as new clsTime(times)
MessageBox.Show(clsTime.FormatTimespan(tm.AvgTime( )))

b) dim tm as new clsTime(times)
MessageBox.Show(tm.AvgTimeAsString())

dim calc as OtherClass
calc.AvgTime = tm.AvgTime
6) The formatting code is duplicated in both functions - should only be
one copy of it, place it in a private function

hth,
Alan.

Nov 21 '05 #4

P: n/a

<th*****@msala.net> wrote in message
news:43**********************@news.newsdemon.com.. .
:
: I have written my first Class and am posting it to this newsgroup with
: hopes that I can get some feedback on what is right and what is wrong
: with it.
: All comments are welcome, but more interested in what would make it
: better. If I have totally missed on what a class is used for please
: inform me.
:
: Thanks,
:
: Thomas

<snip>

: Public Class clsTime
:
: #Region "Constructors"
:
: Public Sub New(ByVal aryTime() As String)
:
: _AvgTime = funAvgTime(aryTime)
: _TimeSpan = funTimeSpan(aryTime)
: _Count = intFirst
:
: End Sub
:
: #End Region

<snip>

-------------------------------------------
You only have one constructor. This will allow a consumer of this class
to create an instance of this class thusly:
Dim obj As New clsTime
In this case, the _AvgTime, _TimeSpan and _Count member variables aren't
being set.
If you don't want this behavior, declare a private parameterized
constructor:

Private Sub New()
End Sub

This will force users to use the other constructor when creating an
instance.
Also, your use of properties in order to limit access to the member
variables is correct. However, you aren't implementing them in a useful
way.
In your constructor, you are setting these values by way of function
calls. As I read those functions, they test for date values, returning
empty strings if the test fails. In your properties however, you are
accepting any value submitted. For example, you code allows the
following:

Dim t() As String
'set appropriate values of t() array here

Dim c As New clsTime(t)
c.AvgTime = "Hello"
c.TimeSpan = "World"
c.Count = -1234
One of the strengths of properties is that they allow you to screen your
values. In this case, if the input value is not a date (or a valid
integer range for Count), you can either throw an exception, substitute

an empty string (or 0 for Count) or exit without updating the member
variables with the invalid string/integer value.
Ralf
--
----------------------------------------------------------
* ^~^ ^~^ *
* _ {~ ~} {~ ~} _ *
* /_``>*< >*<''_\ *
* (\--_)++) (++(_--/) *
----------------------------------------------------------
There are no advanced students in Aikido - there are only
competent beginners. There are no advanced techniques -
only the correct application of basic principles.
Nov 21 '05 #5

P: n/a
Alan,

Thanks for all the comments

1) Naming (minor)
Dubious on the naming clsTime.
- The newer naming standards have moved away from the Hungarian
notation. Pretty much everything is a class now
*****************************************
I will keep the naming convention in mind
*****************************************
2) Ctors
- If the class is intended to work upon date/time values then pass
them in as such rather than strings.
************************************************** *******************
The array is filled by looping through some database records
I was not sure what would happen if I came across a blank
date field, so I passed them as a string and used the isdate
to decide if the value would be used in the the function
If is not a date then it is skipped.
************************************************** *******************

- If possible, an arraylist is simpler for working with. It will
not impact this class but when generating the inputs an arraylist will
probably make things easier.
************************************************** *
I will read about the arraylist I have never used
one
************************************************** *

- Calculate the properties when needed instead of generating them at
time of construction. You may not always use both
- you will need to either store a reference to the incoming
date/times (if you want the average/span to update if you change any of
the original values)
or, copy the values if you want a snapshot view.
************************************************** *******************
Not sure about this. The records are gathered and the array
is filled at the start of the sub. The class is declared later and
the values of each of the properties are used as strings in the
construction of the text of the report. I guess readonly properties
would be fine. I will have to read on how to make them readonly
************************************************** *******************
2)
Private _Count As Integer
Private datFirst As Date
Private datLast As Date
Private intFirst As Integer = 0
Private intMinute As Integer = 0
Private intSecond As Integer = 0

do not need to exist at the class level
***********************************************
is it better to declare them in each function
or declare them once at the class level. Seems
easier to me to declare them at the class level
***********************************************
3) Properies

Count can be generated from the time list, not need to store
It should be read only
AvgTime and TimeSpan should both return TimeSpan

They both should be read-only
If allowing the inputs to be modified then you will want to
recalc them every time (no variables to store them,
Private _AvgTime As String
Private _TimeSpan As String
both disappear) or else you calc them only the first time they
are used, store them in the class and each subsequent time, return the
stored value.
************************************************** ***********
Ok, I am very confused on all of the property stuff. I will
have to do some more reading on it.
************************************************** ***********
4) Functionality Questions?

What are the pre-reqs for the incoming data?
************************************************** *
The incoming array basically has not pre-reqs. It
is read from a group of database records.
************************************************** *
Are they all guaranteed to be valid times?
************************************************** *
Nope, they are not checked untion the isdate()
funcion in the class.
************************************************** *
if no, what do we do with the invalid times?
************************************************** *
The if isDate in the function ignores the invalid
times.
************************************************** *
if yes, why are you testing IsDate()?
**************************************************
answered above
**************************************************
Are they guaranteed to be in time sequence?
**************************************************
No, that is why I use the Array.Sort function
*************************************************
if no, then funTimeSpan() will not give the correct value - you
need to find the first and last time and then find the difference
**************************************************
After the Array.sort method the array will be in
order, then the datFirst and datLast variable
capture the correct dates to determine the timespan
**************************************************
if yes, then why are we using System.Math.Abs(), t+1 - t will
always be non-negative
************************************************** *
I was not sure if the Array.Sort method would alway
put the array in accending order. If it was sorted
descending the function would still work using the
System.Math.Ads function.
************************************************** *
5) Add a shared function to the class that will create a string from a
TimeSpan that will do the correct formatting for you or you can put in
AsString() versions of the properties.
By providing only String() versions of the output you limit
how/where it can be used. Any class that wants to use the values needs
to be able to parse the string back to a timespan so it needs to know
in what format you have stored it. Which may change.

a) dim tm as new clsTime(times)
MessageBox.Show(clsTime.FormatTimespan(tm.AvgTime( )))

b) dim tm as new clsTime(times)
MessageBox.Show(tm.AvgTimeAsString())

dim calc as OtherClass
calc.AvgTime = tm.AvgTime

*****************************************
Lost me here
*****************************************

6) The formatting code is duplicated in both functions - should only be
one copy of it, place it in a private function

************************************************** ***
This makes sense, I will move all the formatting to
a separate function
************************************************** ***

You did just what I wanted.

Thanks,

Thomas

--
Posted via NewsDemon.com - Premium Uncensored Newsgroup Service
------->>>>>>http://www.NewsDemon.com<<<<<<------
Unlimited Access, Anonymous Accounts, Uncensored Broadband Access
Nov 21 '05 #6

P: n/a
First step is to take a long bath and scrub off all of those old C coding
skills. Classes are not functional elements in the same manner as in a C API
library call or such. Instead they are objects that should represent some
sort of physical entity of your system. First ask your self why do you need
to calculate Time averages and such. These operations should belong to some
sort of entity that you can describe. Such as Mail messages, appointments,
.... Something real and tangible. When you discover what these entities are
then you know what classes you should be describing. These time calculations
should be methods that are exposed by these classes. Not seperate objects.

"th*****@msala.net" wrote:

I have written my first Class and am posting it to this newsgroup with hopes
that I can get some feedback on what is right and what is wrong with it.
All comments are welcome, but more interested in what would make it better.
If I have totally missed on what a class is used for please inform me.

Thanks,

Thomas
'************************************************* *************************************
'************************************************* *************************************
'**** This class accepts one parameter. This parameter is passed as a
Date/Time ****
'**** array. From this variable the Class calculates and exposes three
****
'**** properties. AvgTime - which is average time between each of the
values ****
'**** in the array. TimeSpan - which is the length of time between the
first ****
'**** and last date/time in the array. Properties are returned as Minute
and ****
'**** Second values only. Count - which is the number of usable Date/Time
groups ****
'**** in the passed array
****
'************************************************* *************************************
'************************************************* *************************************

Public Class clsTime

Private _AvgTime As String
Private _TimeSpan As String
Private _Count As Integer
Private datFirst As Date
Private datLast As Date
Private intFirst As Integer = 0
Private intMinute As Integer = 0
Private intSecond As Integer = 0

#Region "Constructors"

Public Sub New(ByVal aryTime() As String)

_AvgTime = funAvgTime(aryTime)
_TimeSpan = funTimeSpan(aryTime)
_Count = intFirst

End Sub

#End Region

#Region "Properties"

Public Property AvgTime() As String
Get
Return _AvgTime
End Get
Set(ByVal value As String)
_AvgTime = value
End Set
End Property

Public Property TimeSpan() As String
Get
Return _TimeSpan
End Get
Set(ByVal value As String)
_TimeSpan = value
End Set
End Property

Public Property Count() As Integer
Get
Return _Count
End Get
Set(ByVal value As Integer)
_Count = value
End Set
End Property

#End Region

#Region "Functions"

Private Function funAvgTime(ByVal aryTime() As String) As String

Dim intAvTime As Integer = 0
intFirst = 0
intMinute = 0
intSecond = 0

Array.Sort(aryTime)

For x As Integer = 0 To aryTime.GetUpperBound(0)
If IsDate(aryTime(x)) Then
intFirst += 1
If intFirst = 1 Then
datFirst = CDate(aryTime(x))
End If
datLast = CDate(aryTime(x))

If intFirst > 1 Then
intAvTime += DateDiff(DateInterval.Second,
CDate(aryTime(x - 1)), CDate(aryTime(x)))
intAvTime = System.Math.Abs(intAvTime)
End If
End If
Next

If intFirst > 0 Then
intAvTime = intAvTime / intFirst
If intAvTime > 60 Then
intMinute = intAvTime \ 60
intSecond = intAvTime Mod 60
Return CStr(intMinute) & ":" & CStr(intSecond)
Else
Return CStr(intAvTime) & " seconds"
End If
Else
Return String.Empty
End If

End Function

Private Function funTimeSpan(ByVal aryTimeSpan() As String) As String

Dim intLength As Integer = 0
intMinute = 0
intSecond = 0

If IsDate(datLast) AndAlso IsDate(datFirst) Then
intLength = DateDiff(DateInterval.Second, datFirst, datLast)
If intLength > 60 Then
intMinute = intLength \ 60
intSecond = intLength Mod 60
Return CStr(intMinute) & ":" & CStr(intSecond)
Else
Return CStr(intLength) & " seconds"
End If
Else
Return String.Empty
End If
End Function

#End Region

End Class

--
Posted via NewsDemon.com - Premium Uncensored Newsgroup Service
------->>>>>>http://www.NewsDemon.com<<<<<<------
Unlimited Access, Anonymous Accounts, Uncensored Broadband Access

Nov 21 '05 #7

P: n/a

<th*****@msala.net> wrote in message
news:43**********************@news.newsdemon.com.. .
:
: Alan,
:
: Thanks for all the comments
<snip>
: ************************************************** *******************
: - If possible, an arraylist is simpler for working with. It will
: not impact this class but when generating the inputs an arraylist will
: probably make things easier.
: ************************************************** *
: I will read about the arraylist I have never used
: one
: ************************************************** *
They are handy. They are sized dynamically so you don't have to worry
about fixed array lenghts or using ReDim statements. That said, they do
have one serious downside in my opinion - they aren't type safe. Your
code ensures everything in the array is a String because you declare it
as such. However, an array list can contain literally any type of
object. If you don't have control over how the array is formed, this may
work against you.
: - Calculate the properties when needed instead of generating them
: at time of construction. You may not always use both
: - you will need to either store a reference to the incoming
: date/times (if you want the average/span to update if you change any
: of the original values)
: or, copy the values if you want a snapshot view.
: ************************************************** *******************
: Not sure about this. The records are gathered and the array
: is filled at the start of the sub. The class is declared later and
: the values of each of the properties are used as strings in the
: construction of the text of the report. I guess readonly properties
: would be fine. I will have to read on how to make them readonly
: ************************************************** *******************
Public Readonly Property PropertyName() As SomeType
Get
Return SomethingHere
End Get
End Property

: 2)
: Private _Count As Integer
: Private datFirst As Date
: Private datLast As Date
: Private intFirst As Integer = 0
: Private intMinute As Integer = 0
: Private intSecond As Integer = 0
:
: do not need to exist at the class level
: ***********************************************
: is it better to declare them in each function
: or declare them once at the class level. Seems
: easier to me to declare them at the class level
: ***********************************************
Member variables are created on the heap when the class is instantiated
and the memory consumed is not released until the garbage collector
runs. This can become have an undesirable impact on memory resources -
if you have a large number of instances of this class, each one of them
will create space for those variables. Since you don't need them outside
of the functions that use them, keep them there. This way, multiple
instances of the class don't use up memory they don't need.
: 3) Properies
:
: Count can be generated from the time list, not need to store
: It should be read only
:
:
: AvgTime and TimeSpan should both return TimeSpan
:
: They both should be read-only
: If allowing the inputs to be modified then you will want to
: recalc them every time (no variables to store them,
: Private _AvgTime As String
: Private _TimeSpan As String
: both disappear) or else you calc them only the first time they
: are used, store them in the class and each subsequent time,
: return the stored value.
: ************************************************** ***********
: Ok, I am very confused on all of the property stuff. I will
: have to do some more reading on it.
: ************************************************** ***********
The primary value of properties from my perspective is that they isolate
your member variables. For example, they can restrict access by making
them read only (as Alan is suggesting). You can also screen the values
submitted as I suggested previously. If you want to allow the variables
to be set by the consumer of the class, you can still prevent invalid
values from being passed in.
<snip>
Ralf
--
----------------------------------------------------------
* ^~^ ^~^ *
* _ {~ ~} {~ ~} _ *
* /_``>*< >*<''_\ *
* (\--_)++) (++(_--/) *
----------------------------------------------------------
There are no advanced students in Aikido - there are only
competent beginners. There are no advanced techniques -
only the correct application of basic principles.
Nov 21 '05 #8

P: n/a
It's better to keep anything private until needed, things should upgrade in
scope as needed (Private,Friend,Public...), less public code to maintain for
any API users the better.

It's bad idea to call methods from a constructor.

Using strings as dates is bad idea, make them all DateTime. If you need to
make an overload to handle this on the method. Otherwise always use the
specific type or create a new object.

I would not use ArrayList as mentioned, API users cant tell what is
contained in them as easily, and there are no constraints on the item types.
ArrayList should be used for simple internal tasks. Anything public should
be a Typed collection, and sometimes a Typed array.
I suggest you see the TimeSpan class in the .NET Framework.

Also do a search for FxCop.Exe , It's program to check for a lot of things
like these for you, and has info. on most problems if finds.

Schneider

<th*****@msala.net> wrote in message
news:43**********************@news.newsdemon.com.. .

I have written my first Class and am posting it to this newsgroup with hopes that I can get some feedback on what is right and what is wrong with it.
All comments are welcome, but more interested in what would make it better. If I have totally missed on what a class is used for please inform me.

Thanks,

Thomas
'************************************************* **************************
*********** '************************************************* **************************
*********** '**** This class accepts one parameter. This parameter is passed as a
Date/Time ****
'**** array. From this variable the Class calculates and exposes three
****
'**** properties. AvgTime - which is average time between each of the
values ****
'**** in the array. TimeSpan - which is the length of time between the
first ****
'**** and last date/time in the array. Properties are returned as Minute
and ****
'**** Second values only. Count - which is the number of usable Date/Time
groups ****
'**** in the passed array
****
'************************************************* **************************
*********** '************************************************* **************************
***********
Public Class clsTime

Private _AvgTime As String
Private _TimeSpan As String
Private _Count As Integer
Private datFirst As Date
Private datLast As Date
Private intFirst As Integer = 0
Private intMinute As Integer = 0
Private intSecond As Integer = 0

#Region "Constructors"

Public Sub New(ByVal aryTime() As String)

_AvgTime = funAvgTime(aryTime)
_TimeSpan = funTimeSpan(aryTime)
_Count = intFirst

End Sub

#End Region

#Region "Properties"

Public Property AvgTime() As String
Get
Return _AvgTime
End Get
Set(ByVal value As String)
_AvgTime = value
End Set
End Property

Public Property TimeSpan() As String
Get
Return _TimeSpan
End Get
Set(ByVal value As String)
_TimeSpan = value
End Set
End Property

Public Property Count() As Integer
Get
Return _Count
End Get
Set(ByVal value As Integer)
_Count = value
End Set
End Property

#End Region

#Region "Functions"

Private Function funAvgTime(ByVal aryTime() As String) As String

Dim intAvTime As Integer = 0
intFirst = 0
intMinute = 0
intSecond = 0

Array.Sort(aryTime)

For x As Integer = 0 To aryTime.GetUpperBound(0)
If IsDate(aryTime(x)) Then
intFirst += 1
If intFirst = 1 Then
datFirst = CDate(aryTime(x))
End If
datLast = CDate(aryTime(x))

If intFirst > 1 Then
intAvTime += DateDiff(DateInterval.Second,
CDate(aryTime(x - 1)), CDate(aryTime(x)))
intAvTime = System.Math.Abs(intAvTime)
End If
End If
Next

If intFirst > 0 Then
intAvTime = intAvTime / intFirst
If intAvTime > 60 Then
intMinute = intAvTime \ 60
intSecond = intAvTime Mod 60
Return CStr(intMinute) & ":" & CStr(intSecond)
Else
Return CStr(intAvTime) & " seconds"
End If
Else
Return String.Empty
End If

End Function

Private Function funTimeSpan(ByVal aryTimeSpan() As String) As String

Dim intLength As Integer = 0
intMinute = 0
intSecond = 0

If IsDate(datLast) AndAlso IsDate(datFirst) Then
intLength = DateDiff(DateInterval.Second, datFirst, datLast)
If intLength > 60 Then
intMinute = intLength \ 60
intSecond = intLength Mod 60
Return CStr(intMinute) & ":" & CStr(intSecond)
Else
Return CStr(intLength) & " seconds"
End If
Else
Return String.Empty
End If
End Function

#End Region

End Class

--
Posted via NewsDemon.com - Premium Uncensored Newsgroup Service
------->>>>>>http://www.NewsDemon.com<<<<<<------
Unlimited Access, Anonymous Accounts, Uncensored Broadband Access

Nov 21 '05 #9

P: n/a

If there is no default ctor declared then one cannot create an instance
without passing in the parameter(s)

There is no need to declare a private default ctor to prevent this
happening.

e.g.

Public Class Widget

Public Sub New( _
ByVal strName As String _
)
Name = strName
End Sub

Public Overrides Function ToString() As String
Return Name
End Function

Public Name As String

End Class
Dim x as new Widget("Gizmo") --- Works fine
Dim y as New Widget --- Doesn't compile.
Gives a 'Argument not specified for parameter 'strName' of 'Public Sub
New(strName As String)'. error
Alan.

Nov 21 '05 #10

P: n/a
I would not use ArrayList as mentioned, API users cant tell what is
contained in them as easily, and there are no constraints on the item types.
ArrayList should be used for simple internal tasks. Anything public should
be a Typed collection, and sometimes a Typed array.

Agreed, a strongly typed collection would be best. Have a look at the
CollectionBase example in the help/walkthroughs.
************************************************* **********************
Not sure about this. The records are gathered and the array
is filled at the start of the sub. The class is declared later and
the values of each of the properties are used as strings in the
construction of the text of the report. I guess readonly properties
would be fine. I will have to read on how to make them readonly
************************************************* **********************

There are two parts to this.

1) Changeable or not. Is sounds as if you are reading a snapshot of
these values and then want to get the calculated values.
You are not going to update the values once read.
You may use them more than once

Public readonly Property AverageTime() as TimeSpan
Get
If (_avgTime is nothing) then
_avgTime = CalcAverageTime()
end if
return _avgTime
End Get
End Proeprty
2) This is a readonly property because it is calculated from the
datetime values and it does not make sense to say that you can set it
to a value.

*********************************************** *
is it better to declare them in each function
or declare them once at the class level. Seems
easier to me to declare them at the class level
*********************************************** *
The goal is more to reduce clutter and ease maintenance than to make
typing easier.
Variables should be declared at the smallest scope possible.
Class level variables should be used only for data that needs to exist
outside / longer than a single method. Anything that can
be declared within a method and then forgotten about when the method
ends should be declared within the method and forgotten about when the
method ends.
************************************************* ***
I was not sure if the Array.Sort method would alway
put the array in accending order. If it was sorted
descending the function would still work using the
System.Math.Ads function.
************************************************* ***


I missed the Array.Sort the first time around, sorry. This imposes
constraints on the order in which you can access the properties. It
should be done as part of the construction rather than within a
specific calculation. They should be independent.

Sort will always work the same way for a set of dates within a given
Culture. Given that, Abs() should not be needed.
Note: This cannot be said for strings and is another good reason to
work with datetime objects rather than strings

e.g.

Dim strings(3) As String
strings(0) = "1/1/2004"
strings(1) = "2/1/2004"
strings(2) = "11/1/2004"

Array.Sort(strings)

dim str as New System.Text.StringBuilder
For Each strDate As String In strings
str.Append(strDate + System.Environment.NewLine)
Next
MessageBox.Show(str.ToString)
gives

1/1/2004
11/1/2004
2/1/2004


Alan.

Nov 21 '05 #11

P: n/a
On 2005-08-23, th*****@msala.net <th*****@msala.net> wrote:

I have written my first Class and am posting it to this newsgroup with hopes
that I can get some feedback on what is right and what is wrong with it.
All comments are welcome, but more interested in what would make it better.
If I have totally missed on what a class is used for please inform
me.
OK, you asked...

'************************************************* *************************************
'************************************************* *************************************
'**** This class accepts one parameter.
I like the class comment. Consider using the same kind of comment on
all public methods and properties. In the next version, intellisense
will start picking those up...
Public Class clsTime

Private _AvgTime As String
Private _TimeSpan As String
Private _Count As Integer
Private datFirst As Date
Private datLast As Date
Private intFirst As Integer = 0
Private intMinute As Integer = 0
Private intSecond As Integer = 0
Pick a naming convention and stick to it. Ideally I should be able to
look at any code snippet and know immediately which variables are local
and which are class-level variables. Here, some have an underscore,
some don't... there's no standard.

It's kind of a bete noire of mine, but why not _AverageTime? It's only
4 more letters. Also, this is subtle but if you had inputs of 2:00 and
3:00 I'd expect _AverageTime to be 2:30. Maybe _AverageTimeSpan and
_TotalTimeSpan are better variable names?

Here's a good exercise. Pretend you didn't write the code and that
you're looking at that variable list for the first time. I can guess at
what _Count means (although it turns out my first guess was wrong, it's
really more of a _ValidCount rather than just a _Count), but I have no
idea what intFirst or intMinute are. If you hadn't seen the code, would
you have the same reaction? If so, then you need better variable names.

#Region "Constructors"

Public Sub New(ByVal aryTime() As String)
I'd mention that I disagree with those who didn't like accepting a
string array in the ctor. I think that's fine.
_AvgTime = funAvgTime(aryTime)
Always consider other people who might need to understand your code when
naming things. I'm not really sure what funAvgTime does. I recognize
CalculateAverageTimeSpan(aryTime) immediately.

#End Region

#Region "Properties"

Public Property AvgTime() As String
Others have mentioned this, but these properties should be readonly and
should probably return a TimeSpan rather than a string.
#Region "Functions"

Private Function funAvgTime(ByVal aryTime() As String) As String

Dim intAvTime As Integer = 0
intFirst = 0
intMinute = 0
intSecond = 0

Array.Sort(aryTime)
That ain't gonna work with strings. There's a bigger point to be made
here. The .Net framework is very, very rich. Instead of doing
calculations yourself, you can usually find a class that will do it for
you (in this case the DateTime class).

For x As Integer = 0 To aryTime.GetUpperBound(0)
If IsDate(aryTime(x)) Then
intFirst += 1
If intFirst = 1 Then
datFirst = CDate(aryTime(x))
End If
datLast = CDate(aryTime(x))

If intFirst > 1 Then
intAvTime += DateDiff(DateInterval.Second,
CDate(aryTime(x - 1)), CDate(aryTime(x)))
intAvTime = System.Math.Abs(intAvTime)
Consider something. What's the Math.Abs doing there? Didn't we sort
the array up top? If the DateDiff comes back as negative, then we have
a serious bug on our hands, because the averages will be wrong. Don't
hide bugs like this. At the very least, you should have an assert here.

And even during release, personally I'd rather show an error message
than silently give the wrong answer. I strongly suspect you added the
Abs because you were getting negative values and didn't know why.
There's a good lesson here.
End If
End If
Next

If intFirst > 0 Then
intAvTime = intAvTime / intFirst
This might be picky, but it's the kind of thing I feel pretty strongly
about. What does the intAvTime variable hold? Sometimes it holds the
sum of the timespans and sometimes it holds the average of the
timespans. IMHO, it's a really bad idea to have a variable represent
two different things.

The first time I looked at the above line, I said to myself "why are we
dividing by the first date variable?" Good variable names and good
function names are an overwhelmingly large part of good code.
Private Function funTimeSpan(ByVal aryTimeSpan() As String) As String

Dim intLength As Integer = 0
intMinute = 0
intSecond = 0

If IsDate(datLast) AndAlso IsDate(datFirst) Then
intLength = DateDiff(DateInterval.Second, datFirst, datLast)
If intLength > 60 Then
intMinute = intLength \ 60
intSecond = intLength Mod 60
Return CStr(intMinute) & ":" & CStr(intSecond)


Personally, I like giving variables the smallest possible scope, and
only declare them when I need to. In this case

If IsDate(datLast) AndAlso IsDate(datFirst) Then
Dim length As Integer = DateDiff(...
If length > 60 Then
Dim minute As Integer = ...

You get the idea. Why declare things at function scope if the whole
function doesn't need them?

You've gotten a lot of comments, but I'd also add that things are private
when they should be, apart from things you didn't know about (like
ReadOnly properties and TimeSpans) the public interface is well-defined,
it's a good level of functionality for a single class and the class
pretty much works. If this is your first Class ever I'd say it's a
pretty good job.

Nov 21 '05 #12

P: n/a

<al*******@users.com> wrote in message
news:11**********************@z14g2000cwz.googlegr oups.com...
:
: If there is no default ctor declared then one cannot create an
: instance without passing in the parameter(s)
:
: There is no need to declare a private default ctor to prevent this
: happening.
<snip>
You are correct. I get hit with this every time. Per the documentation:
"If a class is defined without any constructors, a public empty
constructor is added by the compiler. However, if they class
includes at least one parameterized constructor, the compiler
will *not* insert an empty constructor."
One of these days, I'll get it right the first time out the door.
Ralf
--
----------------------------------------------------------
* ^~^ ^~^ *
* _ {~ ~} {~ ~} _ *
* /_``>*< >*<''_\ *
* (\--_)++) (++(_--/) *
----------------------------------------------------------
There are no advanced students in Aikido - there are only
competent beginners. There are no advanced techniques -
only the correct application of basic principles.
Nov 21 '05 #13

P: n/a
Again, just to be sure you saw it. I don't know how much every here uses it
but:

"Also do a search for FxCop.Exe , It's program to check for a lot of things
like these for you, and has info. on most problems if finds.
"

Schneider
"dfoster" <df*****@woofix.local.dom> wrote in message
news:sl********************@localhost.localdomain. ..
On 2005-08-23, th*****@msala.net <th*****@msala.net> wrote:

I have written my first Class and am posting it to this newsgroup with hopes that I can get some feedback on what is right and what is wrong with it.
All comments are welcome, but more interested in what would make it better. If I have totally missed on what a class is used for please inform
me.


OK, you asked...

'************************************************* **************************
*********** '************************************************* **************************
*********** '**** This class accepts one parameter.


I like the class comment. Consider using the same kind of comment on
all public methods and properties. In the next version, intellisense
will start picking those up...
Public Class clsTime

Private _AvgTime As String
Private _TimeSpan As String
Private _Count As Integer
Private datFirst As Date
Private datLast As Date
Private intFirst As Integer = 0
Private intMinute As Integer = 0
Private intSecond As Integer = 0


Pick a naming convention and stick to it. Ideally I should be able to
look at any code snippet and know immediately which variables are local
and which are class-level variables. Here, some have an underscore,
some don't... there's no standard.

It's kind of a bete noire of mine, but why not _AverageTime? It's only
4 more letters. Also, this is subtle but if you had inputs of 2:00 and
3:00 I'd expect _AverageTime to be 2:30. Maybe _AverageTimeSpan and
_TotalTimeSpan are better variable names?

Here's a good exercise. Pretend you didn't write the code and that
you're looking at that variable list for the first time. I can guess at
what _Count means (although it turns out my first guess was wrong, it's
really more of a _ValidCount rather than just a _Count), but I have no
idea what intFirst or intMinute are. If you hadn't seen the code, would
you have the same reaction? If so, then you need better variable names.

#Region "Constructors"

Public Sub New(ByVal aryTime() As String)


I'd mention that I disagree with those who didn't like accepting a
string array in the ctor. I think that's fine.
_AvgTime = funAvgTime(aryTime)


Always consider other people who might need to understand your code when
naming things. I'm not really sure what funAvgTime does. I recognize
CalculateAverageTimeSpan(aryTime) immediately.

#End Region

#Region "Properties"

Public Property AvgTime() As String


Others have mentioned this, but these properties should be readonly and
should probably return a TimeSpan rather than a string.
#Region "Functions"

Private Function funAvgTime(ByVal aryTime() As String) As String

Dim intAvTime As Integer = 0
intFirst = 0
intMinute = 0
intSecond = 0

Array.Sort(aryTime)


That ain't gonna work with strings. There's a bigger point to be made
here. The .Net framework is very, very rich. Instead of doing
calculations yourself, you can usually find a class that will do it for
you (in this case the DateTime class).

For x As Integer = 0 To aryTime.GetUpperBound(0)
If IsDate(aryTime(x)) Then
intFirst += 1
If intFirst = 1 Then
datFirst = CDate(aryTime(x))
End If
datLast = CDate(aryTime(x))

If intFirst > 1 Then
intAvTime += DateDiff(DateInterval.Second,
CDate(aryTime(x - 1)), CDate(aryTime(x)))
intAvTime = System.Math.Abs(intAvTime)


Consider something. What's the Math.Abs doing there? Didn't we sort
the array up top? If the DateDiff comes back as negative, then we have
a serious bug on our hands, because the averages will be wrong. Don't
hide bugs like this. At the very least, you should have an assert here.

And even during release, personally I'd rather show an error message
than silently give the wrong answer. I strongly suspect you added the
Abs because you were getting negative values and didn't know why.
There's a good lesson here.
End If
End If
Next

If intFirst > 0 Then
intAvTime = intAvTime / intFirst


This might be picky, but it's the kind of thing I feel pretty strongly
about. What does the intAvTime variable hold? Sometimes it holds the
sum of the timespans and sometimes it holds the average of the
timespans. IMHO, it's a really bad idea to have a variable represent
two different things.

The first time I looked at the above line, I said to myself "why are we
dividing by the first date variable?" Good variable names and good
function names are an overwhelmingly large part of good code.
Private Function funTimeSpan(ByVal aryTimeSpan() As String) As String
Dim intLength As Integer = 0
intMinute = 0
intSecond = 0

If IsDate(datLast) AndAlso IsDate(datFirst) Then
intLength = DateDiff(DateInterval.Second, datFirst, datLast)
If intLength > 60 Then
intMinute = intLength \ 60
intSecond = intLength Mod 60
Return CStr(intMinute) & ":" & CStr(intSecond)


Personally, I like giving variables the smallest possible scope, and
only declare them when I need to. In this case

If IsDate(datLast) AndAlso IsDate(datFirst) Then
Dim length As Integer = DateDiff(...
If length > 60 Then
Dim minute As Integer = ...

You get the idea. Why declare things at function scope if the whole
function doesn't need them?

You've gotten a lot of comments, but I'd also add that things are private
when they should be, apart from things you didn't know about (like
ReadOnly properties and TimeSpans) the public interface is well-defined,
it's a good level of functionality for a single class and the class
pretty much works. If this is your first Class ever I'd say it's a
pretty good job.

Nov 21 '05 #14

This discussion thread is closed

Replies have been disabled for this discussion.