How To Setup An Effective Refactoring Process of a Heavy Database Interface

This story is about pain, agony, and denial of ready-made solutions. It is also about changes that improve the code’s readability and help the development team stay happy. The object of this post is an interface that helps a program communicate with a database.

Disclaimer: If we would use various ORMs such as Gorm in this project, most probably we would not face this issue, yet, we decided to write our implementation, so this created the issue and therefore this post.

The problem with this interface was its size — 130+ methods in one single interface! That’s a lot of methods and that is not what SOLID interface should look like. And since we develop in Go, we have to know (and follow) one of the Go proverbs which are:

The bigger the interface, the weaker the abstraction. © Rob Pike

The further we developed the project, the heavier this interface grew and soon it became clear that to continue the development with fewer bugs, less time spent understanding the code, and more comfort, this interface should be refactored. We as a team could not use this interface with flexibility. We could not tell from the first glance what it does since it does everything. And that forced me to start its refactoring. Which is what I want to share with you.

Step 0: Cover Code (API) with Tests before any Refactoring

This is crucial, since dealing with interfaces, abstractions, and refactoring without tests that cover API logic makes no good. I would say that it can do exactly opposite — bring a lot of problems to your code since every change you make to the interface will result in 20+ files being changed. And if you do not have solid tests, there is a high chance you break something or create bugs. Please, be cautious!

Step 1: Imagine What the Result Would Look Like

I decided to concentrate on common aggregates that my project deals with. After some time of thinking and looking through the entire list of functions, I outlined how the future interface would look like and this is what I came up with:

// IStorage represents database methods
type IStorage interface {
	User() User
	Profile() Profile
	Agreement() Agreement
	AgreementChanges() AgreementChanges
	Milestone() Milestone
	Contact() Contact
	Notification() Notification
	Payout() Payout
	WebhookEvent() WebhookEvent
	Referral() Referral
	VerificationCode() VerificationCode
	Feedback() Feedback
	PlatformTransfer() PlatformTransfer
}

This would allow writing the following code instead of just reference one of 130+ methods from the interface:

user, err := api.Storage.GetUserByID(ctx, userID)
err := api.Storage.DenyAgreement(ctx, agreement)
err := api.Storage.UpdateUserDeviceToken(ctx, 
model.UserDeviceToken{...})

After refactoring:

user, err := api.Storage.User().Get(ctx, userID)
err := api.Storage.Agreement().Deny(ctx, agreement)
err := api.Storage.User().UpdateDeviceToken(ctx, 
model.UserDeviceToken{...})

As you can see, this interface consists of multiple smaller interfaces each based on certain aggregates (users, agreements, etc.) and doing something with that aggregate. Reading such constructions is a much better experience and at the same time, it is more convenient since if you ever need to do anything with a user, you know where to search for the right methods and consider if they even exist.

Step 2: Implement Changes Step by Step

Having interface with 130+ methods makes it very complicated to do refactor in “once and for all” style. There are so many changes that turn every merge request into 50+ files changes. So, the next step, therefore, should be breaking the interface step by step, one aggregate after another and committing those changes often to make small, understandable merge request and making sure everything still works as expected (remember step 0!) For this I first break down methods into small portions

// IStorage represents database methods
type IStorage interface {
	// User() User
	// Profile() Profile
	// Agreement() Agreement
	// AgreementChanges() AgreementChange
	// Milestone() Milestone
	// Contact() Contact
	// Notifications() Notification
	// Payout() Payout
	// WebhookEvents() WebhookEvent
	// Referrals() Referral
	// VerificationCodes() VerificationCode
	// Feedback() Feedback
	// PlatformTransfe() PlatformTransfer

	// this is just a temporary change not to 
break all the functionality. Will reimplment this
	// for the version mentioned above as a 
next step with just a small chucks under 
improvements
	User
	Profile
	Agreement
	AgreementChange
	Milestone
	Contact
	Notification
	Payout
	WebhookEvent
	Referral
	VerificationCode
	Feedback
	PlatformTransfer
}

// Notification inferface contains methods for 
notifications manipulation
type Notification interface {
	CreateNotification(ctx context.Context, n 
*model.Notification) (model.Notification, error)
	GetNotification(ctx context.Context, id 
int64) (model.Notification, error)
	GetUnreadNotificationsCount(ctx 
context.Context, receiverID int64) (int, error)
	ListNotification(ctx context.Context, 
userID int64) ([]model.Notification, error)
	SetNotificationAsRead(ctx 
context.Context, id int64) error
	ResetUnreadNotifications(ctx 
context.Context, receiverID int64) error
}

So, I created several sub-interfaces and stated that my IStorage interface implements them all. That did not change the code much but laid an important preparatory brick to what I wanted to do next, which is essentially replace my sub-interfaces with separate interfaces with their separate methods in CRUD fashion, adding missing methods & uniting those which are of the same nature.

I added new structs as the implementation of those interfaces and replaced old methods with a new one with VSCode search & replace all features.

// IStorage represents database methods
type IStorage interface {
	Feedback() Feedback

	// this is just a temporary change not to 
break all the functionality. Will reimplment this
	// for the version mentioned above as a 
next step with just a small chucks under 
improvements
	User
	Profile
	Agreement
	AgreementChange
	Milestone
	Contact
	Notification
	Payout
	WebhookEvent
	Referral
	VerificationCode
	PlatformTransfer
}

func (s *Storage) Feedback() Feedback {
	return &FeedbackClient{cfg: s.cfg, 
logger: s.logger, pool: s.pool}
}

// Feedback inferface contains methods for 
feedback manipulation

type Feedback interface {
	Create(ctx context.Context, f 
*feedback.Feedback) (*feedback.Feedback, error)
}

type FeedbackClient struct {
	cfg    config.Config
	logger *logrus.Entry
	pool   *pgxpool.Pool
}

As a result, I have changed all IStorage sub-interfaces to its interfaces and made all functions more intuitive & understandable. Yes, it took me a while to do that, yet, the result is worth it.

Step 3: Clean up

Bulk edits with find -> replace all helps save time, but it also creates some side effects where you can rename not relevant function or the logs messages might get a bit silly and hard to understand. This what happened to me after refactoring.

err = api.Storage.Profile().Update(ctx, user)
if err != nil {
  api.Logger.WithFields(logrus.Fields{"err": 
err}).Error("Update failed")
}

When this gets logged we get the message “Update failed” which might not be so clear to us at first glance. Yes, there is a “handler” param that indicates where exactly it happened, but this might not be enough to determine the exact spot. So, my advice here would be to go ahead and review the project’s log messages and other components that might get affected by the refactoring.

Side Effects from Refactoring

During this process, it became clear that we use a lot of different methods for the same thing (like updating items in various fields in various methods). This creates code smell and in the future, we would avoid that anti-pattern.

Also, I have found some functions that were not “fit” in certain places. For example when agreements were highly dependent on referrals in the update. That made no sense after I begin refactoring. Having this “God” interface allowed code to use it, but refactoring showed how terrible it was and made living with this code impossible. I have to re-write some functions to create more natural, intuitive functionality.

Yes, we have to write some more code, yet, this approach gives us way more to code’s readability, maintainability, and simple design. I would do it after all.

Conclusion

If you have a project with a very heavy database interface and you see that it continues to grow, consider refactoring as soon as possible since it would just get worse.

When refactoring, always check tests first, then decide what the end solution should look like and do partial updates, one component at a time. This will keep your project alive, will decrease chances of introducing new bugs, will keep code reviewers from anger, and keep you happy about the progress.

After refactoring large interfaces always check for ‘side effects”. Basically
If you ever face the same issue, please share your solutions in the comments, since I am really curious about what else I might have done. Also, please share your thoughts on what I could (and still can) do to make it even better. Any feedback is more than welcome.

Previously published at blog.maddevs.io.