Programming practices question

[Maciek]

I've got this question regarding programming practices. I'm designing
Newsletter module in my WebApp and I'm greenhorn in programming.
There's a stored procedure which adds a subscriber to a DB. It outputs
subscriberID (uniqueidetifier) if it succeeds to add them to the
database and return value of "0". When the subscriber already exists
but hasn't activated/confirmed their account it returns null for
subscriberID and return value of "1". If the subscriber exists and is
fully activated it returns null for subscriberID and return value of
"2". Here is the SP code:

create procedure Newsletter_PendingSubscriberAdd
@email varchar(255),
@name varchar(255) = null,
@company varchar(255)= null,
@subscriberID uniqueidentifier output
as
set nocount on
if exists (select email from Newsletter_Subscribers where email =
@email) return 2
if exists (select email from Newsletter_PendingSubscribers where email
= @email) return 1

select @subscriberID = newid()

declare @err int
insert into Newsletter_PendingSubscribers (subscriberid, email, [name],
company) values (@subscriberID, @email, @name, @company)
select @err = @@error if @err <> 0 return @err
return 0

set nocount off
GO


Now in my data layer in my App I'm trying to write method to add the
subscriber to the db. My problem is how and where to react on different
values returned from sql. The method is accessing the procedure and now
I check the Return Value. If it's 0 the method returns subscriberID and
if it's not -- it returns the Return Value.

Public Function Add(ByVal Email As String, ByVal Name As String,
ByVal Company As String)
Dim rowsAffected As Integer
Dim result As Integer
Dim parameters As SqlParameter() = { _
New SqlParameter("@email", SqlDbType.VarChar, 255), _
New SqlParameter("@name", SqlDbType.VarChar, 255), _
New SqlParameter("@company", SqlDbType.VarChar, 255), _
New SqlParameter("@subscriberID", SqlDbType.UniqueIdentifier)}

parameters(0).Value = IIf(Len(Trim(Email)) = 0, DBNull.Value,
Email)
parameters(1).Value = IIf(Len(Trim(Name)) = 0, DBNull.Value,
Name)
parameters(2).Value = IIf(Len(Trim(Company)) = 0, DBNull.Value,
Company)
parameters(3).Direction = ParameterDirection.Output

' I am using this DBObject class for accessing data:
http://www.devx.com/vb2themax/Tip/19480
result = RunProcedure("Newsletter_PendingSubscriberAdd",
parameters, rowsAffected)
If result = 0 Then
Return CStr(parameters(3).Value)
Else
Return CInt(result)
End If

End Function


But this is, I think, bad design. Mainly because this method may return
two different kinds of data types: string for uniqueidetifier
(subscriberID) and Integer for RetVal. So I would need to propagate up
the App layers unspecified (until runtime) data type. This can cause
many problems I think. The other approach I can think of would be
returning subscriberID when subscriber was not in db and throwing and
propagating an Exception if they're already in DB:

If result = 0 Then
Return CStr(parameters(3).Value)
Else
Throw Ex("My custom exception")
End If


But I've read somewhere that we shouldn't fool around with exceptions
if the result (from DB in this case) was EXPECTED. And this is expected
behaviour. The exceptions are for unexpected situations, I think. Hey,
but what do I know, I'm a rookie! Is there the third (right) way of
doing this? Where do I make my design mistakes? Any advice appreciated!

Thanks
Best regards
Maciek

 

[Mark]

The Add function should return a result relevant to it's purpose - so it
should return true or false

The subscriber ID (and possibly "result") should be returned as an OUT
parameter

--
Best regards
Mark Baldwin

I've got this question regarding programming practices. I'm designing
Newsletter module in my WebApp and I'm greenhorn in programming.
There's a stored procedure which adds a subscriber to a DB. It outputs
subscriberID (uniqueidetifier) if it succeeds to add them to the
database and return value of "0". When the subscriber already exists
but hasn't activated/confirmed their account it returns null for
subscriberID and return value of "1". If the subscriber exists and is
fully activated it returns null for subscriberID and return value of
"2". Here is the SP code:

create procedure Newsletter_PendingSubscriberAdd
@email varchar(255),
@name varchar(255) = null,
@company varchar(255)= null,
@subscriberID uniqueidentifier output
as
set nocount on
if exists (select email from Newsletter_Subscribers where email =
@email) return 2
if exists (select email from Newsletter_PendingSubscribers where email
= @email) return 1

select @subscriberID = newid()

declare @err int
insert into Newsletter_PendingSubscribers (subscriberid, email, [name],
company) values (@subscriberID, @email, @name, @company)
select @err = @@error if @err <> 0 return @err
return 0

set nocount off
GO


Now in my data layer in my App I'm trying to write method to add the
subscriber to the db. My problem is how and where to react on different
values returned from sql. The method is accessing the procedure and now
I check the Return Value. If it's 0 the method returns subscriberID and
if it's not -- it returns the Return Value.

Public Function Add(ByVal Email As String, ByVal Name As String,
ByVal Company As String)
Dim rowsAffected As Integer
Dim result As Integer
Dim parameters As SqlParameter() = { _
New SqlParameter("@email", SqlDbType.VarChar, 255), _
New SqlParameter("@name", SqlDbType.VarChar, 255), _
New SqlParameter("@company", SqlDbType.VarChar, 255), _
New SqlParameter("@subscriberID", SqlDbType.UniqueIdentifier)}

parameters(0).Value = IIf(Len(Trim(Email)) = 0, DBNull.Value,
Email)
parameters(1).Value = IIf(Len(Trim(Name)) = 0, DBNull.Value,
Name)
parameters(2).Value = IIf(Len(Trim(Company)) = 0, DBNull.Value,
Company)
parameters(3).Direction = ParameterDirection.Output

' I am using this DBObject class for accessing data:
http://www.devx.com/vb2themax/Tip/19480
result = RunProcedure("Newsletter_PendingSubscriberAdd",
parameters, rowsAffected)
If result = 0 Then
Return CStr(parameters(3).Value)
Else
Return CInt(result)
End If

End Function


But this is, I think, bad design. Mainly because this method may return
two different kinds of data types: string for uniqueidetifier
(subscriberID) and Integer for RetVal. So I would need to propagate up
the App layers unspecified (until runtime) data type. This can cause
many problems I think. The other approach I can think of would be
returning subscriberID when subscriber was not in db and throwing and
propagating an Exception if they're already in DB:

If result = 0 Then
Return CStr(parameters(3).Value)
Else
Throw Ex("My custom exception")
End If


But I've read somewhere that we shouldn't fool around with exceptions
if the result (from DB in this case) was EXPECTED. And this is expected
behaviour. The exceptions are for unexpected situations, I think. Hey,
but what do I know, I'm a rookie! Is there the third (right) way of
doing this? Where do I make my design mistakes? Any advice appreciated!

Thanks
Best regards
Maciek

 

[Maciek]

Hi Mark,

The Add function should return a result relevant to it's purpose - so it
should return true or false

The subscriber ID (and possibly "result") should be returned as an OUT
parameter

Thanks for the thought. Well, I think it's a good point! But I need to
be able to check why the subscriber wasn't added to the db and
depending on results I want to show different messages to the WebApp
user. When subscriber is still pending (didn't activated his account),
maybe there were problems with sending an email to them with account
activation link. So when I know that the subscriber is still pending i
can allow them to send again account activation email. So with this
approach I would probably need some kind of property with status or
something... Maybe like this?

dim result as boolean
result = subscriber.Add()
if not result then

select case subscriber.AddStatus
case 1
' subscriber pending -- allow to resend email
case 2
' subscriber already activated
end select

end if

But in this case one logical operation is splited into Add method and
AddStatus property and it seems odd to me. but again, I'm a rookie.

Well, I've just came up with this thought. Maybe Add method should:
- set value to private variable/property (null or not null - then we
can access subscriberid with ID property - when the result is 0)
- return result: 0, 1, 2 (yes, it's against yours: " Add function
should return a result relevant to it's purpose" - but in this case it
would be an exception to the rule)

Then in case of 0 the subscriber ID would be accessed with ID property,
which i need anyway. so it's not an 'overheat' like AddStatus property.

dim subscriberID as string
dim result as Integer
result = subscriber.Add()
if result = 0 then
subscriberID = subscriber.ID
else
select case result
case 1
' subscriber pending -- allow to resend email
case 2
' subscriber already activated
end select
end if


What do you think about it?

Regards
Maciek