Import Yahoo Contacts in C#

I have the following code for importing Yahoo contacts from an address book, but it imports only the top 30 contacts, not all of the contacts.

Anyways, I think another alternative (and better) way of doing it is by using another latest Yahoo API.

```
private const string _addressBookUrl = "http://address.mail.yahoo.com/allcontacts";
//private const string _addressBookUrl = "http://address.yahoo.com/yab/us/Yahoo_ab.csv?loc=us&.rand=1671497644&A=H&Yahoo_ab.csv";
private const string _authUrl = "https://login.yahoo.com/config/login?";
private const string _loginPage = "https://login.yahoo.com/config/login";
private const string _userAgent = "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3";

[TestMethod()]
public void Yahoo()
{
List lst = new List();
lst = GetYahooContacts("UserId", "Password");
Assert.IsNotNull(lst);
}

public List GetYahooContacts(string _username, string _pass)
{
try
{
WebClient webclient = new WebClient();
webclient.Headers[HttpRequestHeader.UserAgent] = _userAgent;
webclient.Encoding = Encoding.UTF8;

byte[] firstResponse = webclient.DownloadData(_loginPage);
string firstRes = Encoding.UTF8.GetString(firstResponse);

NameValueCollection postToLogin = new NameValueCollection();
Regex regex = new Regex("type=\"hidden\" name=\"(.?)\" value=\"(.?)\"", RegexOptions.IgnoreCase);
Match match = regex.Match(firstRes);
while (match.Success)
{
if (match.Groups[0].Value.Length > 0)
{
postToLogin.Add(match.Groups[1].Value, match.Groups[2].Value);
}
match = regex.Match(firstRes, match.Index + match.Length);
}

postToLogin.Add(".save", "Sign In");
postToLogin.Add(".persis

These are bad variable names, they should be descriptive of what kind of values they contain:

string[] tmp1 = cookie.Split(',');
string[] tmp2 = var.Split(';');

Moreover, tmp1 isn't even necessary since you only use it once.

Also avoid things like oYahooContacts, just name them yahooContacts.

Don't abbreviate, e.g. _pass. It isn't necessary and only makes code harder to read.

I don't think ParseYahooResponse is descriptive enough. Sure, that method does parse the response, but more importantly it returns a list of contacts.

Variables should be camelCase:

List ContactList = new List();
List Contacts

I don't think YahooContacts is a good class name, for one it is a plural. It can also be simplified by using Auto-Implemented Properties, e.g.

public string ContactID {get;set;}

Things like private const string _addressBookUrl belong in a separate class IMHO, or perhaps in a config file.

Why do you name your parameter _username and then assign it to the variable login? Plus in your test method you call it "UserId".

I'm not a fan of this:

postToLogin.Add(".save", "Sign In");
postToLogin.Add(".persistent", "y");
postToLogin.Add("login", login);
postToLogin.Add("passwd", _pass);

This reeks of magic strings. I would be tempted to make this a class of its own, which then can be converted to a NameValueCollection, and things like ".save" would be constants in that class, perhaps.

At least I'd move the whole postToLogin logic to its own class, have it parse the response and return a NameValueCollection.

Actually, that goes for most of the code. I'd move some code from GetYahooContacts at least to separate methods, certainly the lines where you create a new cookie. The whole method is rather hard to figure out, for instance I didn't quite understand why you assigned to webclient.Headers[HttpRequestHeader.Cookie] multiple times until I noticed webclient.UploadValues(_authUrl, postToLogin); happens between those two assignments.

I'd much prefer it if the logic in that method was distributed over several shorter method, so the logic becomes clearer, e.g. do a download, parse the response/create the postToLogin NameValueCollection, do an upload, create the cookie, do a download, get the contact details.

StackExchange Code Review Q#75278, answer score: 6