Example of refactoring with proxy pattern

Today I came across a good refactoring example, extracting a cross cutting concern using proxy pattern. First let me explain what I’m trying to achieve here and then dig into the problem and how I solve it.

The example code

In our app, we have a class to wrap any queries we do to Firestore. This makes it a lot easier to mock this out when we test various components. The code I will refactor looks roughly like this:

class FirestoreQueries {
  static const _collection = 'article-feed';

  Future<T> tracePerformance<T>(Future<T> future, String name, Map<String, dynamic> params) {
    final trace = FirebasePerformance.instance.newTrace(name);
    trace.start();
    params.forEach((key, value) => trace.putAttribute(key, value));
    return future.whenComplete(() => trace.stop);
  }


  Future<DocumentSnapshot> getArticleById(String id) {
    return tracePerformance(Firestore.instance.document("$_collection/$id").get(), "get_article", {});
  }

  // .... more methods of a similar style
}

As we are having some performance issues, we’ve added in the tracePerformance helper. This wraps futures and logs their execution time to Firebase. Now, as the culprit is identified, let’s talk about the crime.

Single responsibility

The single responsibility principle tells us classes should have only one reason to change. This makes our code much easier to maintain.

In the code above, if we want to change the way we trace or even remove the tracing, should the queries class need to be changed and re-tested? The queries themselves haven’t changed so why re-test it? The tracing is a cross cutting concern, it’s something the queries doesn’t care about and something we might want to turn off/on.

Proxy pattern to the rescue

The code in the query class doesn’t depend at all on the tracing. It doesn’t bring anything to the functionality of the class. It is logging. We can take this one concern out. Using a proxy we move the responsibility to trace performance out to a separate class.

Clean, separated and best of all if we keep the exact same interface. No client needs to change (aside from where the class is instantiated, I’ll show this).

class FirestoreQueries {
  static const _collection = 'article-feed';

  Future<DocumentSnapshot> getArticleById(String id) {
    return Firestore.instance.document("$_collection/$id").get();
  }

  // .... more methods of a similar style
}

// separate file
Future<T> tracePerformance<T>(Future<T> future, String name, { Map<String, dynamic> params = const {} }) {
  final trace = FirebasePerformance.instance.newTrace(name);
  trace.start();
  params.forEach((key, value) => trace.putAttribute(key, value));
  return future.whenComplete(() => trace.stop);
}

class TracedFirestoreQueries implements FirestoreQueries {

  final FirestoreQueries _wrappedQueries;

  TracedFirestoreQueries(this._wrappedQueries);

  @override
  Future<DocumentSnapshot> getArticleById(String id) {
    return tracePerformance(_wrappedQueries.getArticleById(id), 'get_article_by_id');
  }

  @override
  Future<QuerySnapshot> getArticlesPublishedBefore(DateTime time) {
    final params = {"published_before": time.toIso8601String()};
    return tracePerformance(_wrappedQueries.getArticlesPublishedBefore(time), 'get_articles_published_before',  params: params);
  }

  @override
  Future<List<ReadArticle>> getUsersReadArticles() {
    return tracePerformance(_wrappedQueries.getUsersReadArticles(), 'get_users_read_articles');
  }
}

Dependency injection from main

Our app uses dependency injection to make testing easier to allow what I will show you next. We don’t use a framework (yet), instead, we pass dependencies directly to classes as they are instantiated in main.dart.

The idea follows a principle Uncle Bob talks about in his presentations on Clean Architecture. That higher-level abstraction objects should not know about lower-level abstraction objects. That’s a lot of fancy words, to (over) simplify: depend on contracts, not on concrete classes.

In our case, the FirestoreQueries object, though concrete, is a contract. Using extends we can create a proxy that looks and talks like the base class but has a different implementation.

How do the clients know which one to use? That’s where the dependency injection comes in. All classes that require the queries class receive this object as a parameter from main.

final firestoreQueries = TracedFirestoreQueries(FirestoreQueries());
final articleRepo = new TimeoutArticleRepository(new FirestoreArticles(firestoreQueries), Duration(seconds: 20));
return AppRoot(
  logger: logger,
  analytics: analytics,
  repository: articleRepo,
);

Conclusion

So this is an actual example of how we can refactor things. I haven’t changed any logic, only moved it, and I’ve reduced file and class sizes, split responsibilities and improved testability.

These code bits were all taken from the real code of the JReader app.

Any thoughts, suggestions, other ways you would do it? Do comment below!