I am new in vb.net as i want to return one more string value that is html_output (given in below code) with errorMessage i tried the tuple but its showing error while calling that function

Public Function GenerateHTMLTemplate(ByVal StatusRecord As DeliveryStatus) As String
    Dim errorMessage As String = String.Empty
    Try
        'fetching HTML Template file path
        Dim hTMLTemplate As String = File.ReadAllText($"{ConfigurationManager.AppSettings("html_template_file_path")}")

        'file path for new generated HTML file 

        Dim **html_output** As String = $"{ConfigurationManager.AppSettings("generated_email_file_path")}"

        hTMLTemplate = hTMLTemplate
        .Replace("{LETTERID}", letterID)
        .Replace("{CID}", cID)

        File.WriteAllText(html_output, hTMLTemplate)
    Catch ex As Exception
        errorMessage = $"Failed to generate the HTML Template due to the following exception: {ex}"

    End Try

    Return (errorMessage)
End Function

CodePudding user response：

Technically you could add a ByRef argument to your signature and set the value at the end of the function (before the return), but I would not suggest doing that in this specific case.

Typically speaking, you do not want to return the error message from a function. If the code runs fine then it returns an empty String, which presumably means that you aren't going to do anything with it.

I would suggest:

1. Get rid of the Try/Catch
2. Return the desired variable from the Function
3. Get rid of the StatusRecord argument (it isn't used)
4. Wrap the call to the Function in a Try/Catch

E.g.:

Public Function GenerateHTMLTemplate() As String
    'fetching HTML Template file path
    Dim hTMLTemplate As String = File.ReadAllText($"{ConfigurationManager.AppSettings("html_template_file_path")}")

    'file path for new generated HTML file
    Dim html_output As String = $"{ConfigurationManager.AppSettings("generated_email_file_path")}"

    hTMLTemplate = hTMLTemplate
        .Replace("{LETTERID}", letterID)
        .Replace("{CID}", cID)

    File.WriteAllText(html_output, hTMLTemplate)

    Return html_output
End Function

Private Sub Foo()
    Dim outputHtml = String.Empty
    Try
        outputHmtl = GenerateHTMLTemplate()
    Catch ex As Exception
        Dim errorMessage = $"Failed to generate the HTML Template due to the following exception: {ex}"
        ' do something with errorMessage?
    End Try
End Sub

CodePudding user response：

I don't see where StatusRecord is actually used, but I'll give you the benefit of the doubt that we're seeing a simplified version of the problem and it matters in your real code.

That out of the way, I would NOT return an error message. I would remove the Try/Catch completely and allow the exception to leak out to the calling code. If it were good enough to always catch the exception in the method where it's thrown we'd still be using the old On Error Resume paradigm.

Public Function GenerateHTMLTemplate(ByVal StatusRecord As DeliveryStatus) As String
    'fetching HTML Template file path
    Dim hTMLTemplate As String = File.ReadAllText(ConfigurationManager.AppSettings("html_template_file_path"))

    'file path for new generated HTML file 
    Dim html_output_path As String = ConfigurationManager.AppSettings("generated_email_file_path")

    hTMLTemplate = hTMLTemplate.
        Replace("{LETTERID}", letterID).
        Replace("{CID}", cID)

    File.WriteAllText(html_output_path, hTMLTemplate)
    Return html_output_path
End Function

It also seems weird to return the path here. Did you want to return the actual HTML? Anyway, wrap the whole function call in a Try/Catch at the points where you call it, like this:

Try
    Dim result As String = GenerateHTMLTemplate(someVariable)

    ' ... everything is fine

Catch ex As Exception
    ' You have an error
End Try     

But if you really want to return two values (again: you should NOT do this), it would look like this:

Public Function GenerateHTMLTemplate(ByVal StatusRecord As DeliveryStatus) As (String, String)
    Dim errorMessage As String = String.Empty
    Try
        'fetching HTML Template file path
        Dim hTMLTemplate As String = File.ReadAllText(ConfigurationManager.AppSettings("html_template_file_path"))

        'file path for new generated HTML file 

        Dim html_output As String = ConfigurationManager.AppSettings("generated_email_file_path")

        hTMLTemplate = hTMLTemplate
        .Replace("{LETTERID}", letterID)
        .Replace("{CID}", cID)

        File.WriteAllText(html_output, hTMLTemplate)
    Catch ex As Exception
        errorMessage = $"Failed to generate the HTML Template due to the following exception: {ex.Message}"

    End Try

    Return (errorMessage, html_output)
End Function

And then you'd call it like this:

Dim result = GenerateHTMLTemplate(someVariable)
If Not String.IsNullOrEmpty(result.Item1)
     ' You have an error in result.Item1
Else
     ' You have html_output in result.Item2
End If

I need to add that Tuples are a relatively recent addition to .Net. If you're running an old version of Visual Studio you may need to update or add Imports System.ValueTuple to make this work. But this is unlikely to be a problem in an environment new enough to support string interpolation.

Note that using Tuples doesn't really save you any work at the call site. You still have to do similar things to check the error, regardless of which mechanism you use: either If/Else or Try/Catch. But if we instead let the exception leak out to the calling code, we save the Try/Catch inside the method, so in addition to being generally better practice that option is also less code overall.