ResourceReaderRefactor


 Raul post in his blog (eXtreme Programming. Net) a few lines of code demonstrating how to read embedded resources from one assembly.


 


http://blogs.clearscreen.com/ragc/archive/2004/06/09/316.aspx


 


Today we have had a lunch (at 16:300 we are in Spain 😉 and we discussed about this post, because I replied looking for a better way to get the resource without the loop, he thought my comment was about performance, and we will never have a big number of resources so the loop is not a performance problem.


 


What I said is about “Clean Code that Works”, not performance, in the code I see too many statements:


 


1.      looking for the resource name with a loop though all the resources


2.      compare the resource name


3.      if found try to load as stream


4.      if not found throw an exception


 


And here is the original code


 


public string GetEmbeddedResourceContent(string resourceFileName)


{


      Assembly myAsembly = Assembly.GetExecutingAssembly();


      string resName = String.Empty;


      foreach(string resourceName in


myAsembly.GetManifestResourceNames())


      {


            if (resourceName.IndexOf(resourceFileName)>=0)


            {


                  resName = resourceName;


                  break;


            }


      }


      if (resName.Length > 0)


      {


            Stream s = myAsembly.GetManifestResourceStream(resName);


            StreamReader sr = new StreamReader(s,


 System.Text.Encoding.GetEncoding(1252));


            return sr.ReadToEnd();


      }


      else


      {


            throw new Exception("The resource can't be found");


      }


}


 


Before to start refactor this method I need a test:


 


[TestFixture]


public class ResourceTextReaderTest


{


      ResourceTextReader reader;


 


      [SetUp]


      public void SetUp()


      {


        reader = new ResourceTextReader();


      }


 


      [Test]


      public void GetEmbeddedResourceContent()


      {


  Assert.AreEqual("Hello", 


reader.GetEmbeddedResourceContent("File.txt"));                  


      }


 


      [Test, ExpectedException(typeof(Exception))]


      public void GetEmbeddedResourceContent_NotFound()


      {


        Assert.AreEqual("Hello", 


reader.GetEmbeddedResourceContent("FileNotFound.txt"));                      


      }


}


 


The fist thing I would change is the method name, and because this is a sample, I added two new tests instead of change the old ones, before writing the new code:


 


[Test]


public void ReadResourceFromAssembly()


{


  Assert.AreEqual("Hello",


reader.ReadResourceFromAssembly("File.txt"));                    


}


 


[Test, ExpectedException(typeof(Exception))]


public void ReadResourceFromAssembly_NotFound()


{


  Assert.AreEqual("Hello", 


reader.ReadResourceFromAssembly("FileNotFound.txt"));                  


}


 


Now I have red bar, so I can start to implement my method.


 


public string ReadResourceFromAssembly(string resourceFileName)


{                


  Assembly a = Assembly.GetExecutingAssembly();            


  string result = string.Empty;                


  Stream s = a.GetManifestResourceStream("ResReader." +


resourceFileName);


  using (StreamReader sr = new StreamReader(s))                  


  {


      result =  sr.ReadToEnd();


  }


  return result;


}


Note the use of the “using” clause to avoid close the stream manually.


 


But I still have a red bar because the test ReadResourceFromAssembly_NotFound expects an exception of type “Exception” and I’m getting a ArgumentNullException because the stream returned is null.


 


For users of my method this exception is not very informative, so I’m going to check if the stream is null, and throw a more explanatory exception.


 


Here it is the final version code (with green bar of couse 😉


 


public string ReadResourceFromAssembly(string resourceFileName)


{                


  Assembly a = Assembly.GetExecutingAssembly();            


  string result = string.Empty;                


  Stream s = a.GetManifestResourceStream("ResReader." +


resourceFileName);


  if (null==s)


  {


      throw new Exception("Resource " + resourceFileName + " not found


in assembly " + a.FullName);


  }


  using (StreamReader sr = new StreamReader(s))                        


  {


      result =  sr.ReadToEnd();


  }


  return result;


}


 


Final Conclusion.


 


Readers probably are thinking if all this work was really  needed, I have reduced the lines of code from 16 to 12 not a big reduction, however this code works in the same way, but the readability has been improved and the complexity has been reduced from 8 to 4 (devMetrics report).


 


Finally the main advantage is that I don’t need to loop to throw the exception, if the returned stream is null is enough to throw.


 


 

Comments (2)

  1. ragc says:

    Great work!

    Admit that I gave you the oportunity to show how to refactor the shit-code that i had wrote… 😉

Skip to main content