HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Getting valid URL from user input

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
usergettinginputvalidfromurl

Problem

I have a dialog which populates my ConnectionStringData object with the URL of the service that I'm trying to connect to. I then consume that URL with the following property:

/// 
    /// Where the Service is located.
    /// 
    public string ServiceLocation
    {
        get
        {
            string fullUrl = ConnectionStringData.Location.Trim();

            if (fullUrl.EndsWith("/"))
            {
                fullUrl += "Routes";
            }
            else
            {
                fullUrl += "/Routes";
            }

            Uri uri = null;

            if(Uri.TryCreate(fullUrl, UriKind.Absolute, out uri))
            {
                return uri.ToString();
            }

            throw new UriFormatException(String.Format("{0} is an invalid url", fullUrl));
        }
    }


Should I be doing work like this in the getter? Can I do this better?

Solution

This piece:

if (fullUrl.EndsWith("/"))
{
    fullUrl += "Routes";
}
else
{
    fullUrl += "/Routes";
}


can be rewritten to:

fullUrl += fullUrl.EndsWith("/") ? "Routes" : "/Routes";


or:

if (!fullUrl.EndsWith("/"))
    fullUrl += "/";
fullUrl += "Routes";


Since you're using the TryCreate method, there's no need to throw an exception. You just have to notify the caller whether the conversion succeeded or not. Plus, throwing an exception in a get statement is bad practice. This results in following:

if (Uri.TryCreate(fullUrl, UriKind.Absolute, out uri))
    return uri.ToString();
//if no succesful creation:
return String.Empty;


If you do want to throw an exception, use a try/catch block and a regular Uri constructor. This should be placed in a method and not in the get statement then:

try
{
    uri = new Uri(fullUrl, UriKind.Absolute);
    return uri.ToString();
}
catch (UriFormatException ex)
{
    throw;
}


My favor would still be with the TryCreate resulting in following final code:

public string ServiceLocation
{
    get
    {
        var fullUrl = ConnectionStringData.Location.Trim();
        fullUrl += fullUrl.EndsWith("/") ? "Routes" : "/Routes";

        Uri uri = null;

        if (Uri.TryCreate(fullUrl, UriKind.Absolute, out uri))
            return uri.ToString();
        return String.Empty;
    }
}

Code Snippets

if (fullUrl.EndsWith("/"))
{
    fullUrl += "Routes";
}
else
{
    fullUrl += "/Routes";
}
fullUrl += fullUrl.EndsWith("/") ? "Routes" : "/Routes";
if (!fullUrl.EndsWith("/"))
    fullUrl += "/";
fullUrl += "Routes";
if (Uri.TryCreate(fullUrl, UriKind.Absolute, out uri))
    return uri.ToString();
//if no succesful creation:
return String.Empty;
try
{
    uri = new Uri(fullUrl, UriKind.Absolute);
    return uri.ToString();
}
catch (UriFormatException ex)
{
    throw;
}

Context

StackExchange Code Review Q#69710, answer score: 5

Revisions (0)

No revisions yet.