Case Study 1: Use Block to Fix Legacy Code

As a developer, we often have to deal with legacy code. By legacy code I mean code without unit tests. In a large code base, there are usually some implicit dependencies. Refactoring is not always feasible. We have to understand what the code might affect. After refactoring, it needs throughly regression testing. It may take longer, and probably also needs other engineers’ help. Before deciding to refactor, we should think about why we want to it, how it will benefit.

A few days ago, I worked on a copy experiment. A copy experiment is an A/B test for a specified translation. It is supposed to take a little time to implement and quickly gather users feedback. However, due to legacy code, the experiment became harder. (Because I could not disclosure the source code, I will use an example to demonstrate the problem and how I solved it.)

The label to change is on a table cell contentView’s subview. When reusing a cell, the label is recreated and added to the subview.

It called -configureWithModel:, and a new label is created by helper. helper is a shared instance. The translation to experiment with is assign to the shared instance right after its initialization. If a user has viewed the table view before receiving the A/B testing configurations from server, he/she would still see the original copy until restarting the app. Users can change languages within our app. If a user switches to a different language, the translation does not change at all. It is a bug!

I could observe the app language change notification, and update the helper.attributeString. However, I do not like putting business logic inside a view. And this particular view is inside each cell. If I do this, there would be tens of instances observing and responding to the language change.

My solution is that let helper observe the language change. It only has one instance. I added a new property to Helper class. It is a block, which returns an NSAttributedString.

When the language changes, it use this block to generate a new string, and update attributeString . Now assign to generateString instead.

I did not change/remove attributeString because it is used in other places. I did not want to spend another hour to find out how they use it.

It is not the best code, the label should not be recreated every time, attributeString and generateString are kind of redundancy. But this solution has no impact on existing code, no impact on performance. What is better, it could be quickly implemented, and does not need extra regression tests. If the A/B result is negative, I could clean up easily. It is a trade off I would take.

We do not want to add more technical debts to our projects. But we also want to spend time on features that will benefit our users and companies. There are always codes to improve in your projects. When to refactor, you should better to consider before taking actions.