In the code I help maintain in my day job, I see a lot of the following code:
try
{
/* code */
}
catch(Exception)
{
}
I see it in several different languages almost daily. It really frustrates me that my colleagues and predecessors did this. I stamp it out ruthlessly.
Here is a great post on why empty catch blocks are bad. Here is a great question and series of answers on StackOverflow about best practices for exception handling.
Once we make a request to Github.com with RestSharp, we get a response, and RestSharp gives us a RestResponse object, with which we can do something with the content. The content will be in the format that we specified when we made the request, either JSON, XML or YAML.
Oh crap, complicated string parsing…
RestSharp to the rescue! We don’t have to worry about parsing the response content, because RestSharp can do it for us.
What we need to do, is model the response content into a POCO (Plain Old Clr Object):
public class User
{
public virtual int Id { get; set; }
public virtual string Login { get; set; }
public virtual string Name { get; set;}
...
}
Note that in this User class, I’ve made the properties virtual, this is not necessary for RestSharp to function correctly, it’s more habit on my part from working with NHibernate; however it does mean you can reuse the same models with NHibernate (if you wanted to do something like store the response in a database, for example).
Then we need to modify our client such that when we execute the request, we instruct RestSharp to construct an instance of the User object. Create the client in the usual way and also create the request in the usual way. The magic is in how the client executes the request:
var request = new RestRequest
{
Resource = string.Format("/user/show/{0}", username),
RootElement = "user"
};
var response = client.Execute<User>(request);
var user = response.Data;
As you can see, the Execute method has a generic overload. Internally, RestSharp detects that because we have used this overload, we want to deserialise the response content into an object of the given type, and it performs the deserialisation and constructs an instance of the object. The way that it does this is by looking at the Content-Type header in the response, and it uses the correct deserialiser depending upon the Content-Type. You can see more detail about this on RestSharp’s Github project pages.
It is really easy to work with the response from your REST request with RestSharp, you can access the raw string content of the response, or deserialise it into a POCO – it’s up to you.
Let’s write a C# API library for github.com.
Github has a REST base API, the details of which are available at develop.github.com. Before continuing, I should point out that there is an existing C# library already available, if you want to use that.
We’ll leverage John Sheehan’s excellent RestSharp library to do most of the heavy lifting.
Before we can really do anything, the first task at hand is to learn how to work with RestSharp, and how we can make a request and receive a response. Fortunately, RestSharp makes it really easy, and there are excellent resources on the project’s wiki page which explain how to do things. Let’s see a quick example:
[Test]
public void MakeBasicRequestToTwitterWithRestSharp()
{
var client = new RestClient("http://api.twitter.com");
client.UserAgent = "TemporalCohesion.TwitterApi";
var request = new RestRequest();
request.Resource = "statuses/public_timeline.json";
var response = client.Execute(request);
if (response.StatusCode == HttpStatusCode.OK)
{
String content = response.Content;
Assert.That(content, Is.Not.Null);
}
else
{
Assert.Fail(response.StatusDescription);
}
}
Here we are making an authenticated request to Twitter, asking for the public timeline in JSON format, but, the same code can easily be applied to github.com. We’ll see more of that later. First we create the client object, and then create the request object – telling it what resource to actually request, and then we execute the request using the client, and then do something with the response. Pretty straightforward, huh?
There is quite a lot that we can do with the Github API at this point, although you’ll quickly see that to do anything really interesting (i.e. modifying your github account, or creating/forking repo’s) requires you to be authenticated. Fortunately for us, Github uses HTTP Basic Authentication, and, RestSharp has a HttpBasicAuthenticator class, and if you put the two together, you can make an authenticated request to Github like this:
var client = new RestClient
{
BaseUrl = "https://github.com/api/v2/json",
Authenticator = new HttpBasicAuthenticator("test", "test")
};
After we set the authenticator, RestSharp takes care of making sure that the headers of the requests we make to Github’s API contain the necessary authentication which identifies us to Github. You’ll note that the BaseUrl here is set to https://github.com/…, so that we can take advantage of SSL. You can can access public data via normal http://, but if we are going to do anything that requires authentication, it will be best if we choose to use https://
Github also provide us with an API key. This is a unique key which identifies us to Github, and give our requests a little bit more security. We can’t set this key with RestSharp, as we need to modify the way the authorization header is generated when we make a request. What we can do though, is to implement our own IAuthenticator to do handle it for us. You can see my implementation up on Github. It’s fairly straightforward – I still wanted to allow basic username:password authentication, as well as username/token: authentication for Github.com. Let’s create a unit test to check everything is working OK.
How do we know if we have made an authenticated request successfully? If you make a request to Github for a user, and you are authenticated as that user, then as well as the standard user response, there is some additional information included in the responses that only you, as the authenticated user, can see. And we can check for this information in the response:
[Test]
public void MakeAuthenticatedRequest()
{
var restRequest = new RestRequest
{
Resource = "/user/show/sgrassie"
};
var client = new RestClient
{
BaseUrl = "http://github.com/api/v2/json",
Authenticator = new GitHubAuthenticator(_secretsHandler.Username, _secretsHandler.Password, true)
};
var response = client.Execute(restRequest);
Assert.That(response.Content, Is.StringContaining("total_private_repo_count"));
}
The unit test is pretty straightforward, I make a request object which will get my user account, I authenticate the request and then execute it. Then I can simply check that the response content contains “total_private_repo_count” – this would only be returned if the request was an authenticated request.
One thing you might notice in the test is that in the GitHubAuthenticator constructor, I get my username and password/api token from the _secretsHandler object, an instance of a class I’ve written which will load my username, password and api key from an XML file. This is so that I don’t have to put my Github.com password and API key into the project’s Github repo, because that would be bad.
Recently I have had to unit test some events in an application I work on. I came up with a workable solution, but I didn’t really like the way it was working, and it just looked ugly. So I did a little digging on Google, and found this helpful question on StackOverflow.
Here is my take it. I’m putting it here so I can find easily find it again. Basically it’s the same, but I’m using a lambda to create my anonymous delegate:
[Test]
public void UnitTestNodeChanged()
{
var receivedEvents = new List<XmlNodeChangedEventArgs>();
var document = new XmlDocument();
document.NodeChanged += ((sender, e) => receivedEvents.Add(e));
document.Load("C:\file.xml");
Assert.That(receivedEvents.Count, Is.EqualTo(1));
}
Nice and short, and to the point. We can test the fact that the event was raised (or not); how many times the event was raised; and, we can test the event arguments.
I like it. Some people may not, but it suits my purposes.
I’m sure that you must have heard about Project Euler, which “is a series of challenging mathematical/computer programming problems that will require more than just mathematical insights to solve”. I have blogged about tackling the Project Euler problems before, and at the time, I developed a simple program to try to automate running the problems.
That was about 18 months ago, I’ve learned a lot since then and I think that it is high time to take a look back at the code and see if I can spot if there is any room for improvement.
Current structure
Let’s have a look at how I structured the project when I set it up. The first thing that you will notice, is that I messed up the package names. It should be “uk.co”, and not “co.uk”, according to the Java language specification. A minor point, and not really a big deal, we can easily sort it out.
Something else I did, was lump everything together into the same project and package – the runner program, the utils (e.g. helper classes that you write as you progress through problems), resources and the problems themselves.
I don’t think that this was necessarily a bad idea at the time, I don’t think that it is inherintly a bad idea now, it’s logical for everything to share the same package. Or should I say, it was logical for everything to share the same package.
I am going to put the code for the problem runner onto Github, but I don’t want to share the answers to the problems. To answer your question, I think that it goes against the point of Project Euler, that of having a set of problems that the inquisitive person can solve with some math and programming. I have also found that the problems also help in getting comfortable with the syntax of a new programming language – after all the solutions to the problems remain the same, however the implementation is subtley different depending on the language being used.
Examining the current code and design, we can immediately identify some changes which we are going to make I’m going to seperate the uk.co.temporalcohesion.euler package into three packages, which are more distinct from each other, but still keep them all in the same project. Why? Well, it is obvious that there are three tasks which we have to manage:
To shed some light on my thinking here, lets examine these in more detail. This will also identify areas of possible improvement in the design.
1. I want someway of consistenly running a problem (or group of problems) to test the solution(s) to (a) Project Euler problem(s). This is basically our console runner application, which doesn’t have to know exactly how this happens, it recieves input, and returns output – how that output is generated is irrelevant to it.
2. I want an easy to use API which I can leverage to easily implement the solution to a Project Euler problem, so that I don’t have to worry about re-writing all the input/output code over and over again. I also want to be able to share this API, without sharing the answers to the problems themselves.
3. I need to have somewhere I can put the answers to the problems, and associated helper classes (Prime number generators, etc) which I can easly backup, and easily run the problems from.
These are sounding a bit like user stories, and I suppose they are. They can be formalised as so:
As a Problem Solver, I want to run the solution, or group of solutions, to a Project Euler problem.
As a problem solver, I want to concentrate implementing the solution to a euler problem, not on implementing input/output for the problem.
As a problem solver, I want a place to develop and store the problems, from which I can run the problems to test the solution.
Fairly concise and simple requirements, which we will revisit later. We still have to examine the code in a little more detail so that we can identify exactly what it is we are going to refactor.
The main class to examine is Euler, in Euler.java. The first to say without even looking at the code, is that the name is fairly awful. Then when we examine the code in more detail, we can see the name is even worse. The class has the main entry point for the program, as well as all the code to actually run the problems. This class is doing everything, there is definately no Seperation of Concerns. It is responsible for accepting user input, loading and running the problems and displaying the output. Wow.
The class is designed around the Command Pattern, and it has worked quite well. Looking at it now, a further two patterns could be used, namely Strategy and Template method.I’m still in two minds about refactoring the design pattern in use, but that discussion can be put off for the time being, as I have other things to concern me.
There are 144 SLoC, not a massive amount, but when you consider the above and what the class should be doing, then it is clearly a bit weighty. There are 7 functions in total, not counting the constructor, not a massive count, but as the SLoC count indicates, some of those functions are a bit long. The worst offender is the following method.
[java]
private void loadClasses() {
InputStream fis = null;
Properties p = new Properties();
try {
fis = ClassLoader
.getSystemResourceAsStream("co/uk/temporalcohesion/euler/resources/problems.properties");
p.load(fis);
Enumeration<?> e = p.propertyNames();
while (e.hasMoreElements()) {
String key = (String) e.nextElement();
String value = (String) p.getProperty(key);
Object o = Class.forName(value).newInstance();
if( ( o != null) && (o instanceof Problem)){
Problem problem = (Problem)o;
problems.put(Integer.parseInt(key), problem);
}
}
}
catch (FileNotFoundException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
} catch (ClassNotFoundException e) {
e.printStackTrace();
} catch (InstantiationException e) {
e.printStackTrace();
} catch (IllegalAccessException e) {
e.printStackTrace();
} catch (ClassCastException e) {
}
finally {
try {
fis.close();
} catch (IOException e) {
e.printStackTrace();
}
}
}
[/java]
Wow, that’s a lot of code for something which is relatively straightforward. It’s not exactly easy to read, and it violoates the Single Responsibility Principle: by loading in and enumerating over a properties file, instantiating new instances of problem classes, and handling all the exceptions which can be thrown. There will definately be some room for improvement there.
Getting back to the main Euler class as a whole, there is an even worse problem…
There are no unit tests!
This is a disastrous situation, because it means I cannot refactor the class with any confidence. The me of 18 months ago has a lot to answer for. I’ve got a bit of work to do. Check back next time to see how I’ve progressed things.