Web application for consuming a REST service

Question

The application is a web application that consumes a REST service.

Web config has the REST URL along with the parameters. REST URL and the values are passed as IDictionary and in Repository I convert the same as meaningful REST URL using BuildCompleteUri(). I use RestSharp to consume REST service.

I am trying to learn so please feel free to share anything that comes to your mind.

Web Config

<add key ="ValidatePassword" value="<url>?id=MyUserId&amp;password=MyPassword"/>

UI Layer

public class BasePage : System.Web.UI.Page
{
 public static IUserRepository UserRepository { get { return new UserRepository(); } }
}

---

public partial class Login : KimbayaBasePage
{
 protected void btnSignIn_Click(object sender, EventArgs e)
 {
 IDictionary<string, object> userValidatePasswordParameters = new Dictionary<string, object>();
 userValidatePasswordParameters.Add(UriParameter.MyUserId, txtUsername.Value.Trim());
 userValidatePasswordParameters.Add(UriParameter.MyPassword, pwdPassword.Value.Trim());
 if (UserRepository.Validate(Uri.UserValidatePassword, userValidatePasswordParameters))
 {
 Response.Redirect("Dashboard.aspx", true);
 }
 }
}

Model

public static class UriParameter
{
 public static string MyUserId = "MyUserId";
 public static string MyPassword= "MyPassword";
}

---

public static class Uri
{
 public static string ValidatePassword = "ValidatePassword";
}

---

public class ResultData
{
 public string Result { get; set; }
}

---

public interface IUserRepository
{
 bool Validate(string clientUri, IDictionary<string, object> uriParameters);
}

---

public static class Extensions
{
 public static string BuildCompleteUri(this string source, IDictionary<string, object> uriParameters)
 {
 var completeUri = ConfigurationManager.AppSettings[source];
 foreach (var uriparameter in uriParameters)
 {
 completeUri.Replace(uriParameter.Key, uriParameter.Value.ToString());
 }
 }
}

Repository Layer

public class UserRepository : IUserRepository
{
 IServiceClient client;
 public UserRepository()
 {
 client = new ServiceClient();
 }
 public bool Validate(string clientUri, IDictionary<string, object> uriParameters)
 {
 var completeUri = clientUri.BuildCompleteUri(uriParameters);
 var xml = client.Get(completeUri);
 bool isValid;
 bool.TryParse(XMLParser.Parse<ResultData>(xml)[0].Result, out isValid);
 return isValid;
 }
}

---

public static class XMLParser
{ 
 public static List<T> Parse<T>(string xml) where T : new()
 {
 XDocument reseponseListResultDoc = XDocument.Parse(xml);
 return (from xnode in reseponseListResultDoc.Element("xml").Elements("record") select Get<T>(xnode.Elements("field"))).ToList();
 }
 private static T Get<T>(IEnumerable<XElement> elements) where T : new()
 {
 T t = new T();
 Type typeOfT = typeof(T);
 foreach (var element in elements)
 {
 var elementName = element.Attribute("name").Value;
 if (typeOfT.GetProperty(elementName, BindingFlags.IgnoreCase | BindingFlags.Public | BindingFlags.Instance) != null)
 {
 PropertyInfo propertyInfo = t.GetType().GetProperty(elementName, BindingFlags.IgnoreCase | BindingFlags.Public | BindingFlags.Instance);
 propertyInfo.SetValue(t, Convert.ChangeType(element.Attribute("value").Value, propertyInfo.PropertyType), null);
 }
 }
 return t;
 }
}

Service Layer

public interface IServiceClient
{
 string Get(string uri);
}

---

public class ServiceClient : IServiceClient
{
 private static IRestClient Client(string uri)
 {
 return new RestClient(uri);
 }
 public string Get(string uri, out string errorMessage)
 {
 errorMessage = string.Empty;
 var request = new RestRequest(Method.GET);
 var response = Client(uri).Execute(request);
 switch (response.ResponseStatus)
 {
 case ResponseStatus.Completed:
 return response.Content;
 default:
 errorMessage = response.ErrorMessage;
 return string.Empty;
 }
 }
}

Sample response for validation:

<xml>
 <record type="result">
 <field name="result" value="True"/>
 </record>
</xml>

• Mathieu Guindon: Does your actual code use 1-space indentation?
• Gopi: @Mat'sMug No, I formatted the code manually for code review.
• Greg Burghardt: I would still format it with 4 spaces per indentation level. This is the common practice and standard for .NET code. If the lines of code become too long, we can always scroll towards the right. No big deal.
• Gopi: Thank you @GregBurghardt :) Can you help me formatting the code? When I "Tab" the cursor moves to next control. Do I need to press "Space" 4 times?

Answer 1

This looks like something that belongs in your application settings:

public static class UriParameter
{
 public static string MyUserId = "MyUserId";
 public static string MyPassword= "MyPassword";
}

In terms of oop, this is a total anti-pattern: not only it's all static (i.e. not involving any objects), but you're exposing public fields, and anyone can access these values from the outside, and modify them because they're not readonly. Not to mention you're publicly exposing the password in a plain-text string.

Also the name of the class doesn't really reveal much about its purpose - looks like something along the lines of CredentialsConfig would be more appropriate.

public class CredentialsConfig
{
 private readonly string _userId;
 private readonly string _password; // not quite secure
 public string UserId { get { return ConfigurationManager.AppSettings["UserId"]; } }
 public string Password { get { return ConfigurationManager.AppSettings["Password"]; } }
}

Not any more secure, but at least you'd be encapsulating the configuration settings, and one needs an instance of the class to retrieve the values.

---

This class is also confusing:

public static class Uri
{
 public static string ValidatePassword = "ValidatePassword";
}

It's wrong, for the same reasons as described above (it's essentially a global variable), but it also has a confusing name that actually clashes with the actual System.Uri class, if you have using System; specified anywhere.

---

You shouldn't Trim() the entered password.

userValidatePasswordParameters.Add(UriParameter.MyPassword, pwdPassword.Value.Trim());

What if the user's password includes 3 leading and 5 trailing spaces?

---

Why are you requiring an object here?

public interface IUserRepository
{
 bool Validate(string clientUri, IDictionary<string, object> uriParameters);
}

It seems you're only ever using string for values... Avoid exposing System.object in your API's.

---

There's too much static-ness overall, oop implies working with objects; your code works mostly with types.

Also, this scares me a little:

public class BasePage : System.Web.UI.Page
{
 public static IUserRepository UserRepository { get { return new UserRepository(); } }
}

I'm not all that familiar with [what I believe is] WebForms / ASP.NET, but if this implies that every single page in your application has an IUserRepository handy, it looks like a misplaced property... and again, why does it need to be static?

That IUserRepository only needs to be where it's being used; having it everywhere feels like You Ain't Gonna Need It (YAGNI).

• Gopi: I am sorry @Mat's Mug , my question did not convey exactly what I am doing. MyUserId and MyPassword are static. These are used to identify where I need to pass the exact values in REST url (web.config). So if I pass a Dictionary as <MyPassword,RealPassword>, the BuildCompleteURI will search for MyPassword in ValidatePassword(web.config and replace it with actual password. At the end the ValidatePassword url will look like "<url>?id=RealID&amp;password=RealPassword" . Hope you got me.
• Gopi: I pass the URI and URI Parameters dynamically using URIparameter and URI class to BuildCompleteUri which in turn will find the exact place in url and replace it with real value. so at the end a complete REST uri will be formed.
• Mathieu Guindon: So... you're using static to make global variables, instead of encapsulating data in objects ;-)