Programming practices question

Discussion in 'ASP .Net' started by Maciek, Aug 11, 2006.

  1. Maciek

    Maciek Guest

    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, Aug 11, 2006
    #1
    1. Advertising

  2. Maciek

    Mark Guest

    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


    "Maciek" <> wrote in message
    news:...
    > 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, Aug 11, 2006
    #2
    1. Advertising

  3. Maciek

    Maciek Guest

    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
     
    Maciek, Aug 11, 2006
    #3
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. vipindeep
    Replies:
    1
    Views:
    417
    Rogue Chameleon
    Oct 22, 2004
  2. Ioannis Vranos

    Mozilla C++ programming practices

    Ioannis Vranos, Sep 19, 2004, in forum: C++
    Replies:
    1
    Views:
    415
    Ivan Vecerina
    Sep 19, 2004
  3. vipindeep
    Replies:
    1
    Views:
    412
    Victor Bazarov
    Oct 21, 2004
  4. swille

    programming best practices

    swille, Nov 7, 2005, in forum: Ruby
    Replies:
    15
    Views:
    205
    Stu Glaser
    Nov 12, 2005
  5. harshal
    Replies:
    9
    Views:
    214
    Pedro Del Gallego
    Mar 11, 2008
Loading...

Share This Page