tags:

views:

215

answers:

6

Suggestion in C# or VB.NET are welcome

I have the following code block. I'm tired of typing the same IF statement to check if the string has any lenght before assignning them to the class properties of Employee.

Is there any better to accomplish this?

        Dim title = txtTitle.Text.Trim
        Dim firstName = txtFirstName.Text.Trim
        Dim lastName = txtLastName.Text.Trim
        Dim middleName = txtMiddleName.Text.Trim
        Dim nativeName = txtNativeName.Text.Trim
        Dim mobile = txtMobile.Text.Trim
        Dim workEmail = txtWorkEmail.Text.Trim
        Dim peronsalEmail = txtPersonalEmail.Text.Trim
        Dim otherContact = txtOtherContact.Text.Trim
        Dim home = txtHome.Text.Trim
        Dim homeDir = txtHomeDir.Text.Trim
        Dim homeExt = txtHomeExt.Text.Trim
        Dim personalAddress = txtAddress.Text.Trim
        Dim office = txtOffice.Text.Trim
        Dim officeDir = txtOfficeDir.Text.Trim
        Dim officeExt = txtOfficeExt.Text.Trim

                   If title IsNot Nothing AndAlso title.Length > 0 Then
            newEmployee.Title = HttpUtility.HtmlEncode(title)
        End If

        If firstName IsNot Nothing AndAlso firstName.Length > 0 Then
            newEmployee.FirstName = HttpUtility.HtmlEncode(firstName)
        End If

        If lastName IsNot Nothing AndAlso lastName.Length > 0 Then
            newEmployee.LastName = HttpUtility.HtmlEncode(lastName)
        End If

        If middleName IsNot Nothing AndAlso middleName.Length > 0 Then
            newEmployee.MiddleName = HttpUtility.HtmlEncode(middleName)
        Else
            newEmployee.MiddleName = Nothing
        End If

        If nativeName IsNot Nothing AndAlso nativeName.Length > 0 Then
            newEmployee.NativeName = HttpUtility.HtmlEncode(nativeName)
        Else
            newEmployee.NativeName = Nothing
        End If

        If mobile IsNot Nothing AndAlso mobile.Length > 0 Then
            newEmployee.MobilePhone = HttpUtility.HtmlEncode(mobile)
        Else
            newEmployee.MobilePhone = Nothing
        End If

        If workEmail IsNot Nothing AndAlso workEmail.Length > 0 Then
            newEmployee.Email = HttpUtility.HtmlEncode(workEmail)
        Else
            newEmployee.Email = Nothing
        End If

        If peronsalEmail IsNot Nothing AndAlso peronsalEmail.Length > 0 Then
            newEmployee.PersonalEmail = HttpUtility.HtmlEncode(peronsalEmail)
        Else
            newEmployee.PersonalEmail = Nothing

        End If

        If otherContact IsNot Nothing AndAlso otherContact.Length > 0 Then
            newEmployee.OtherContact = HttpUtility.HtmlEncode(otherContact)
        Else
            newEmployee.OtherContact = Nothing
        End If

        If home IsNot Nothing AndAlso home.Length > 0 Then
            newEmployee.Home = HttpUtility.HtmlEncode(home)
        Else
            newEmployee.Home = Nothing
        End If

        If homeDir IsNot Nothing AndAlso homeDir.Length > 0 Then
            newEmployee.HomeDir = HttpUtility.HtmlEncode(homeDir)
        Else
            newEmployee.HomeDir = Nothing

        End If

        If homeExt IsNot Nothing AndAlso homeExt.Length > 0 Then
            newEmployee.HomeExtension = HttpUtility.HtmlEncode(homeExt)
        Else
            newEmployee.HomeExtension = Nothing
        End If

        If office IsNot Nothing AndAlso office.Length > 0 Then
            newEmployee.Office = HttpUtility.HtmlEncode(office)
        Else
            newEmployee.Office = Nothing
        End If

        If officeDir IsNot Nothing AndAlso officeDir.Length > 0 Then
            newEmployee.OfficeDir = HttpUtility.HtmlEncode(officeDir)
        Else
            newEmployee.OfficeDir = Nothing
        End If

        If officeExt IsNot Nothing AndAlso officeExt.Length > 0 Then
            newEmployee.OfficeExtension = HttpUtility.HtmlEncode(officeExt)
        Else
            newEmployee.OfficeExtension = Nothing
        End If

        If personalAddress IsNot Nothing AndAlso personalAddress.Length > 0 Then
            newEmployee.Address = HttpUtility.HtmlEncode(personalAddress)
        Else
            newEmployee.Address = Nothing
        End If
A: 

you can use a function..

newEmployee.title = ifNotNull(title);
....
....
newEmployee.Office = ifNotNull(office);

public string ifNotNull(string)
{
if(string.Length >0)
return HttpUtility.HtmlEncode(title);
else 
return "";
}
Raze2dust
That will change the value of newEmployee.title. I think he means to leave it alone if the form field is empty.
egrunin
+2  A: 

There are some bad things here, like not declaring your variables as string, but I'll answer your question:

private sub mycopy(value as string, ref target as string)
    value = value.trim
    if value.length > 0 then
        target = HttpUtility.HtmlEncode(value)
    end if 
end sub

mycopy(txtTitle.Text, ref title)
mycopy(txtFirstName.Text, ref firstName)

This is safe even when there is an existing value for title that you don't want changed --which I assume is what you want, otherwise why are you bothering to check?

You can go even further:

private sub mycopy(ctrl as TextBox, ref target as string)
    dim value as string = ctrl.Text.trim ' note declaration as string
    if value.length > 0 then
        target = HttpUtility.HtmlEncode(value)
    end if 
end sub

' use it this way:
dim title as string
dim firstName as string

mycopy(txtTitle, ref title)
mycopy(txtFirstName, ref firstName)
egrunin
A: 

One possibility is to use a helper function, pseudo-code:

def setField (oldField, possibleVal):
    if possibleVal.Length > 0:
        return HttpUtility.HtmlEncode (possibleVal)
    return oldField

newEmployee.Title = setField (newEmployee.Title, title)
newEmployee.FirstName= setField (newEmployee.FirstName, firstName)
:
... and so on.
paxdiablo
...python? Sure, I guess.
Joel Coehoorn
could use DLR to call it. =)
RPM1984
It's pseudo-code guys, as per my comment. The answer is to use a helper function. If the OP can't write one or translate pseudo-code, that's a _different_ issue.
paxdiablo
+6  A: 

I'm tired of typing the same IF statement to check if the string has any lenght

Then don't. IF the newEmployee properties don't already have a significant value (and with a variable name like "newEmployee", they shouldn't), you can just call the function and the result is the same regardless.

If they might already have a significant value you don't want to overwrite, you have several options. You can hide the logic behind a simple helper function, or you can use reflection to iterate over the properties. But to me this mostly calls out as something that needs to be re-written because it will ultimately store html-encoded data in the database. And that's wrong.

ASP.Net labels and many other controls will html-encode data you assign to them for you. And some day you might want this data for a platform where you don't want to html-encode (a report writing tool, for example). Keep the data pure. Handle html encoding at the presentation level at output time.

Either way, you should declare your variables with an explicit type (String, in this case), and you should use the String.IsNullOrEmpty() function rather than just checking the length.

Joel Coehoorn
I agree (and was typing a similar answer). +1
RPM1984
Just as a note: he's using `TextBox` controls, so they'll never be `null`.
egrunin
Why should he declare the variables with an explicit type? Is local type inference a bad thing now?
Jim Mischel
@Jim - somehow I don't think he's figuring on type inference here. It's a clarity thing. In C# you can use var more freely, because then it's obvious you mean to use inference. In VB.Net, you should use a type with the declaration unless you need type inference, because the Dim keyword is the same regardless.
Joel Coehoorn
@Jim but mostly this code just screams of someone who's spent some time with classic asp and just needs to get used to the way things work in .Net some more, and that means thinking in terms of stronger types. This really also answers @egrunin 's critique. It's about building good habits.
Joel Coehoorn
I just added IsNot Nothing AndAlso in the IF statements. Visual Basic 2008 introduces the use of type inference to determine the data types of local variables declared without an As clause http://msdn.microsoft.com/en-us/library/ke6sh835%28v=VS.90%29.aspx
Narazana
A: 

Loop through the all controls on the form, determine if eachcontrol is a textbox and check if length is empty. You can use the String.IsNullOrEmpty() function. This way, you might need to use at most 1 or 2 if statements.

Mamta Dalal
A: 

writing a function (or sub in vb) still requires you to type lots of function calls.

try something like the following (code untested, only pseudo-coding just now):

foreach (Control c in form/containerObject.Controls) {
    if ("TextBox" == c.GetType()) {
        TextBox tb = c as TextBox;
        if (tb.Length > 0) {
            newEmployee/targetObject.Title = HttpUtility.HtmlEncode(tb.Text);
        }
    }
}

or something like that :)

good luck.

alien052002
He would need the object's properties to be in some sort of a collection if he's going to employ such a method and he would need a way to map the control to the property. Perhaps a dictionary? Also, tip: eliminate some of your code by being more explicit about what gets into the foreach. `foreach (TextBox tb in container.Controls.OfType<TextBox>())` will eliminate your need to test the control's type and then cast it. If you *are* going to test the type, consider `if (c is TextBox)` instead of what you're doing (which isn't correct anyway).
Anthony Pegram
:) thanks for the tip.
alien052002
whoops... sorry. enter sends in comment too quick. intent of the _pseudocode_ is that it's not meant to be fully correct, just as a thought process with a little sample. but your tip is correct and appreciated.
alien052002