4
\$\begingroup\$

This is one of the (many) utilitarian classes I have that I use with my ASP.NET sites.

The basis of this one is to make gathering certain tokens that I use periodically, to make URL's more friendly.

It's pretty simple, and should be quite obvious. I use it to make URL's a little more friendly. (/Id/SomeKey/SomeValue/OtherKey/OtherValue/) It returns Id and creates a dictionary for the remaining pairs of Key and Value.

public class RouteUtilities
{
    public static Dictionary<string, string> GetRouteKeyValuePairs(RouteValueDictionary dataTokens, int startIndex)
    {
        Dictionary<string, string> routeItems = new Dictionary<string, string>();
        List<string> segments = GetRouteSegments(dataTokens);

        for (int i = startIndex; i < segments.Count; i += 2)
        {
            routeItems.Add(segments[i], segments[i + 1]);
        }

        return routeItems;
    }

    public static List<string> GetRouteSegments(RouteValueDictionary dataTokens) => (List<string>)dataTokens["FriendlyUrlSegments"];

    public static bool GetIdAndKeys(RouteValueDictionary dataTokens, out int id, out Dictionary<string, string> keys)
    {
        if (dataTokens.Count > 0)
        {
            Dictionary<string, string> routeItems = GetRouteKeyValuePairs(dataTokens, 1);
            List<string> routeSegments = GetRouteSegments(dataTokens);

            if (routeSegments.Count > 0)
            {
                id = Convert.ToInt32(routeSegments[0]);
                keys = GetRouteKeyValuePairs(dataTokens, 1);
                return true;
            }
        }

        id = 0;
        keys = null;
        return false;
    }
}
\$\endgroup\$
3
\$\begingroup\$

In GetIdAndKeys(), you do the default initialization of the out variables just before the default return. I find it is usually better to initialize these variables at the top of the method because many methods will have multiple early return conditions, as well as a default return value. Doing the default initialization at the beginning of the method prevents needing to do it in multiple places, and aids in consistency if the specific method does not have this problem.

public static bool GetIdAndKeys(RouteValueDictionary dataTokens, out int id, out Dictionary<string, string> keys)
{
    id = 0;
    keys = null;

    if (dataTokens.Count > 0)
    {
        // re-assign and return
    }

    return false;  // use default values
}
\$\endgroup\$
3
\$\begingroup\$

I must first say that I am an hater of the out functionality because it makes your code look awful. Given that, I would change your GetIdAndKeys method to the following, if you don't want to create a class for the result...:

public static Tuple<int, Dictionary<string, string>> GetIdAndKeys(RouteValueDictionary dataTokens)
{
    if (dataTokens.Count > 0)
    {
        Dictionary<string, string> routeItems = GetRouteKeyValuePairs(dataTokens, 1);
        List<string> routeSegments = GetRouteSegments(dataTokens);

        if (routeSegments.Count > 0)
        {
            return Tuple.Create(
                Convert.ToInt32(routeSegments[0]), 
                GetRouteKeyValuePairs(dataTokens, 1)
            );
        }
    }
    return null;
}
\$\endgroup\$
2
  • \$\begingroup\$ While this is a fine answer, unfortunately I cannot change all the code that depends on this functionality to be guaranteed as it is now. Plus, I prefer out parameters as it follows the TryParse style better. \$\endgroup\$ – Der Kommissar Nov 27 '15 at 20:15
  • \$\begingroup\$ @EBrown Yep When I said I hate out it also applyies to TryParse, especially when they simply could return a nullable int for the int "tryparse", for example... \$\endgroup\$ – Bruno Costa Nov 27 '15 at 20:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.