How can I improve the following code?

private const string OAuthTokenUri = "oauth2/token";

private static string GetOAuthTokenUri(string uriAddress)
   var uri = new Uri(uriAddress);
   var baseUri = uri.GetLeftPart(UriPartial.Authority);
   var s = new Uri(new Uri(baseUri), OAuthTokenUri);
   return s.ToString();

I call GetOAuthTokenUri() like:

var oauthTokenUri = GetOAuthTokenUri("http://www.example.com/mypage/default.aspx");

As a result I'm getting


Please help to improve GetOAuthTokenUri

  • \$\begingroup\$ So what's your issue? Isn't it working like you expect it to? \$\endgroup\$ – BCdotWEB Mar 9 '16 at 11:59
  • \$\begingroup\$ All works but I want to improve it. This is CodeReview, just for review but not for fix bugs \$\endgroup\$ – Roman Marusyk Mar 9 '16 at 12:00
  • \$\begingroup\$ I don't really see the point of posting a fairly short method and asking for improvements. I'd say that s is a bad variable name and I don't see the point of assigning it (can't you return new Uri(new Uri(baseUri), OAuthTokenUri).ToString());), but there's not much of substance here to warrant a review. \$\endgroup\$ – BCdotWEB Mar 9 '16 at 12:12

Maybe you can do it like this:

private const string OAuthTokenUri = "/oauth2/token";

private static string GetOAuthTokenUri(string uriAddress)
   Uri result = null;
   return Uri.TryCreate(new Uri(uriAddress), OAuthTokenUri, out result) ? result.ToString() : string.Empty;

Please note the slash I have added to OAuthTokenUri.

  • 2
    \$\begingroup\$ Why return string.Empty? Why even use Uri.TryCreate to avoid throwing an exception? \$\endgroup\$ – BCdotWEB Mar 9 '16 at 12:16
  • \$\begingroup\$ Why can't it return string.Empty? Why must it throw an exception? I don't think it's something we should discuss in this question. \$\endgroup\$ – lya Mar 9 '16 at 13:07
  • \$\begingroup\$ Because now you need to check whether you get back null or even worse string.Empty. IMHO this is a case where I'd want to get an exception to be thrown. \$\endgroup\$ – BCdotWEB Mar 9 '16 at 13:34
  • \$\begingroup\$ Well, if an exception is thrown you may also have to handle it. Otherwise the whole program may stop working when an invalid $uriAddress is passed to this function, which IMHO is not a robust design. So do you prefer to handle exceptions or to check the return value? I prefer the latter. \$\endgroup\$ – lya Mar 9 '16 at 14:37

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.