Revision a3df9a9f-e56c-4751-addc-44e6ac43cc4a - Code Review Stack Exchange

The strings `"success"` and `"error-codes"` should be `const string`.

----------

Why is the construction of `postParameters` inside `using (WebClient client = new WebClient())`? On that same topic: considering `client` isn't used after it has filled `response`, why keep it open? Basically, only `response = client.UploadValues(ReCaptchaValidator._googleApiEndpoint, postParameters);` should be inside `using (WebClient client = new WebClient())` (with `byte[] response` defined outside of it).

You also don't need the empty argument at the end of `NameValueCollection()`.

----------

Can `remoteIp` be an empty string? Otherwise, why not use `string.IsNullOrEmpty` instead of `if (remoteIp != null)`?

----------

There's no need to do this:

 string result = ReCaptchaValidator._bodyDivInclude;
 result = result.Replace("%SITEKEY%", _reCaptchaSiteKey);

Just do it in one line:

 string result = ReCaptchaValidator._bodyDivInclude.Replace("%SITEKEY%", _reCaptchaSiteKey);

----------

I'm not a fan of an enum with a plural as a name: `ReCaptchaErrors`. Especially since you then named the field `_errors`, which suggests a collection. Same for `Errors`.

----------

Why do you [use `JavaScriptSerializer` and not Json.NET][1]?

----------

You can reduce the indentation by re-thinkign your if-else blocks:

 if (!deserializedJson.ContainsKey("success"))
 {
 	return false;
 }
 
 _success = deserializedJson["success"];
 _errors = ReCaptchaErrors.None;
 
 if (!deserializedJson.ContainsKey("error-codes"))
 {
 	return true;
 }

----------

It's customary to use `string.Empty` instead of `""`: `string errorEnumName = "";`.

----------

Move `errorWord[0].ToString().ToUpper() + errorWord.Substring(1);` to a method called `Capitalize` and rewrite it slightly:

 private string Capitalize(string s)
 {
 	if (string.IsNullOrEmpty(s))
 	{
 		return string.Empty;
 	}
 	return char.ToUpper(s[0]) + s.Substring(1);
 }

That way your `for` loop can become:

 string errorEnumName = errorWords.Aggregate(string.Empty, (current, errorWord) => current + Capitalize(errorWord));

 [1]: http://stackoverflow.com/a/18314625/648075