Skip to main content

Posts

Showing posts with the label Code refactoring

Code Review: Class Comments

Let's talk in this post about code comments, around the following example. /// <summary> /// ... summary comments ... /// Note: If you change environment make sure you clear this file,as it will contain cached configuration /// </summary> public class ConfigurationPersister { public T GetConfiguration<T>() { ... } public void PersistConfiguration<T>(T configuration) { ... } internal bool HasConfiguration<T>() { ... } private string CreateConfigFileName(string fileName) { ... } private void CheckConfigDirectory() { ... } } Tags First thing that we might notify is the 'NOTE' part. For this kind of scenarios we should use custom tags like '<remarks>'. In this way the other developer that is using the ConfigurationPersister (class), can see more clearly any remarks. /// <summary> /// ... summary comments ... ...

[Post Event] Refactoring and Clean Code Workshop, Sibiu, January 26, 2016

This week I had the opportunity to have a 3 hours workshop at Lucian Blaga University  where I talked about Refactoring and Code Smells in general. We also took a look on Visual Studio 2015 features that help us to refactor our code. Below, you can find the slides from my presentation. The code source I will not share because is ugly and smelly (smile). Radu vunvulea refactoring&amp;code smells from Radu Vunvulea See you next time!

Refactoring in the Maintenance phase

In this blog spot I would like to talk about the maintenance phase of an application, especially from a developer perspective. Usually a software product has the following life cycle: Requirements definition System and Software Design  Implementation and Unit Testing Integration and System Testing Operation and Maintenance At the last phase, you already have a working product that was deliver to the client and is used by users. A big problem at this step is the technical depth that not only exist but will increase with every bug that you fix. When a bug is found, almost all the people recommend to touch only the part of the application where the issues was found and fix the problem with the minim amount of changes. The problem with this approach is related to technical depth and the garbage that you need to pull with you. What is happening with the dirty fixes that are done at this step? Who and when you will refac. this parts of the application and clean the code. ...

[Code refactoring] Multiple Threads - Locks and Mutex

Each of us know what is lock and what its purpose is. It is used to block a critical section of code – the statements from the lock block will be executed only by one thread at the same time. public class Foo { FooConnection con; public void OpenConnection() { ... con.Open(); } public void CloseConnection() { ... con.Close(); } public void Write(...) { ... con.Write(...); } public object Read() { ... ... con.Read(); ... } } ... Foo foo = ...; foo.OpenConnection(); foo.Read(); foo.Read(); ... foo.Read(); foo.RWRWRWR..(); foo.CloseConnection(); Being in a multi-thread environment problems with multiple access to the same resources appeared, information started to be corrupted and many more. For example, multiple read and write started to execute at the same time and having only one connection (cursor) the information retrieved was corrupted. Because there was only one instance of Foo, locks were added to Read a...

[Code refactoring] Releasing resources that you still need

Let’s look over the following code and see if we can find the bug. When the Callback method is called, the _callbackAction field is null all the time (except 2-3 exception). public class FooReader: IDisposable { private ExternalComResource _externalResource; private Action<Foo> _callbackAction; public void Start(Action<Foo> callbackAction) { lock (this) { _callbackAction = callbackAction; _externalResource = new ExternalComResource(); ... _externalResource.Callback += Callback; _externalResource.Start(1); } } public void Stop() { StopAndReleaseResources(); } private void Callback(object source, EventArgs e) { ... StopCapture(); ... if (_callbackAction != null) { _callbackAction.Invoke(someData); } ... } public void Dispose() { StopAndReleaseResources(); } private void StopAndReleaseR...

[Code refactoring] HttpClient status code

Last night I had some time to look over a project that is on CodePlex. There I found the following code: using (var httpClient = new HttpClient()) { httpClient.BaseAddress = fooUrl; FooMessage message = new FooMessage() { ... } Task<HttpResponseMessage> response = httpClient.PostAsJsonAsync("api/foo", message); response.Wait(); if (response.Result.StatusCode != HttpStatusCode.OK) { throw new Exception("..."); } } What do you see strange in this code? Well, there is a check if the return code was 200 (OK) and if not, throw an exception. This is not the wrong way to go, but you should know that HttpClient already contains a method that can check if the return code was 200 (OK). This method will throw an exception automatically if the return code is not the expected one. The above code could be written in the following way: using (var httpClient = new HttpClient()) { httpClient.BaseAd...

[Code refactoring] ThreadStatic and NULL checks

ThreadStatic is used when you have content that needs to be static per thread. For example we can have a field that needs to be static per thread and not per application. In the example below we can see that _message field is decorated with ThreadStatic attribute. Each thread will have a different instance of this field. public class Foo { [ThreadStatic] private static string _messageBuffer; } Great, until now we are good. But scrolling some lines below we find the following code: public class Foo { [ThreadStatic] private static string _messageBuffer; public void DoSomeAction(string input) { if ( _messageBuffer == null ) { _messageBuffer = string.Empty; } _messageBuffer += input; } public void DoMoreAction(string input) { if ( _messageBuffer == null ) { _messageBuffer = string.Empty; } _messageBuffer += " " + input; } ... } This is not good. Why would you like to check at each call if the field is ...

[Code refactoring] From Method to Property

Looking over a project I found the following code: public class AzureLocalStorageTraceListener : XmlWriterTraceListener { public AzureLocalStorageTraceListener() : base(Path.Combine(AzureLocalStorageTraceListener.GetLogDirectory().Path, "WebService.svclog")) { } public static DirectoryConfiguration GetLogDirectory() { DirectoryConfiguration directory = new DirectoryConfiguration(); directory.Container = "wad-tracefiles"; directory.DirectoryQuotaInMB = 10; directory.Path = RoleEnvironment.GetLocalResource("WebService.svclog").RootPath; return directory; } } My attention was attracted by two things. First, was that we had a method called GetLogDirectory that returns the directory configuration for the logs. This method don’t has any custom logic there, only read a configuration from a specific location and retrieves an object populated with specific information. In this case we could have a property called LogDirectoryC...

[Code refactoring] Class name and namespace with the same name (deadly sin)

Looking over some ‘old code’ (3-4 years old) I discovered a class nested under a namespace with the same name. For example we have a class called Foo and the namespace above is also called Foo App.Foo.Foo In that moment I told to myself something is not okay and we need to do something to change this. The good part was that this class was in internal class, used by other subcomponents – no public API there. I don’t know how I missed this problem until now. The fix was very simple, renaming the namespace to a more suitable name. Let’s see why we shouldn’t have a class and a namespace with the same name. First of all you will have problem referring the namespace and the class. You will have to use the full namespace for the class or an alias. This can create confusion to developers, especially because they will never know when the class is referring or what the alias is. You can have different standards to name an alias but the scope is not to hack the system. Namespace are used to...

[Code refactoring] Error Codes

One of my colleges found the following code: public class BaseFooException : System.Exception { public int ErrorCode { get; set; } public string ResourceMessageKey { get { return string.Format("error_{0}", ErrorCode); } } } public class CustomFooException : BaseFooException { public CustomFooException() { ErrorCode = 5; } } The error code was introduce to manage the resources of UI. For each specific error code we had an “error_[code]” in the resources file. When we look over this for the first time we could say that it is okay and the implementation looks good. But, if we look more dipper we can observe that we introduce information related to UI in the all the core components. When an exception from a core component is throw, the component don’t needs to know at that level that some resources are mapped to that error. In this happy case, each exception type has a different exception code. We cannot have two exception refer...