Code Refactoring: My Personal Experience and Lessons Learned
Lessons in Refactoring for Improved Quality and Readability
Table of contents
No headings in the article.
In this article, I will be discussing the implementation of a car inventory management system for a car-selling shop. The shop owner, John, needs a way to track the cars in his inventory, attach prices to them, and keep track of sales. Using my knowledge of object-oriented programming in Go, I will be building simple classes for the "Car," "Product," and "Store" objects. These classes will have the necessary attributes and methods to manage John's car inventory and sales.
Original user story and specifications
John has just opened up his car selling shop, to sell different cars. He gets the cars he needs to sell from different people and they all bring them to him.
He needs to manage the list of cars he has, attach a price to them and put them on display to be sold, basically, John needs an inventory to manage cars & to manage sales. For instance,
He needs to see the number of cars that are left to be sold
He needs to see the sum of the prices of the cars left
He needs to see the number of cars he has sold
Sum total of the prices of cars he has sold
A list of orders for the sales he made
Using the knowledge of OOP in Go, Build simple classes for the following “objects”
Car
Product
Store
The Car class can have any car attributes you can think of.
The Product class should have attributes of a product i.e (the product, quantity of the product in stock, and price of the product). A car is a product of the store, but there can be other products so the attribute of the car can be promoted to the Product. The Product class should have methods to display a product, and a method to display the status of a product if it is still in stock or not.
The Store class should have attributes like
Number of products in the store that are still up for sale
Adding an Item to the store
Listing all product items in the store
Sell an item
Show a list of sold items and the total price
This is not a CLI or web app, the idea is to see how you can think through the problem-solving process using all the knowledge we have gathered in all our lessons. It is a challenge to mainly see how you can think as a programmer. Your implementation will be reviewed line by line
Let's get started...
First, I analyzed the original specification line by line and found something I was confused about:
The Product class should have to display a product, and a method to display the status of a product if it is still in stock or not.
Yeah, am referring to that part in bold. In real life, only the store gets to know if a product is in stock. For example, John just sold one car to Paul. Paul's car can't have a tag to tell if John still has more cars in his store, so it's only ideal for buyers to ask John or go check out his store!
So my initial solution did everything in the specifications (added minor tweaks) with elaborate implementation and actual simulation of how the store works.
Check out the initial implementation here: https://github.com/ukane-philemon/gstore/blob/cac92a98cd6d469718ee889e84c555632535bad3/main.go
Fast-forward to a few weeks later, I had to refactor the codebase. So what changes did I make?
I will walk you through my most recent iteration of the solution in the rest of this article.
Types in my types.go file:
package main
import (
"encoding/hex"
"fmt"
"time"
)
type (
// Product is a product in a Store.
Product interface {
// ID returns the unique ID of the product.
ID() productID
// Type returns the product type.
Type() string
// Product returns the underlying product.
Product() *product
// DisplayName returns the display name of the product.
DisplayName() string
// Price returns the price of the product.
Price() float64
// Display prints information about product.
Display()
// Images returns a list of image urls of the product.
Images() []string
// IsValid checks if a product is valid and returns true if it is valid.
IsValid() bool
}
// order is a buy request from a buyer.
order struct {
id orderID
name string
amountPaid float64
shippingAddress string
products []Product
}
)
// productID is the unique ID of a product.
type productID [16]byte
var zeroProductID productID
func (pi productID) String() string {
return hex.EncodeToString(pi[:])
}
func (pi productID) IsZero() bool {
return pi == zeroProductID
}
// orderID is the unique ID of an order.
type orderID [12]byte
var zeroOrderID orderID
func (oi orderID) String() string {
return hex.EncodeToString(oi[:])
}
func (oi orderID) IsZero() bool {
return oi == zeroOrderID
}
// product implements the Product interface.
type product struct {
id productID
name string
price float64
productType string
category string
description string
images []string
specifications map[string][]string
lastUpdated *time.Time
createdAt *time.Time
}
// ID returns the unique ID of the product.
func (p *product) ID() productID {
return p.id
}
// Type returns the product type.
func (p *product) Type() string {
return p.productType
}
// Product returns the underlying product.
func (p *product) Product() *product {
return p
}
// DisplayName returns the display name of the product.
func (p *product) DisplayName() string {
return p.name
}
// Description returns brief information about the product.
func (p *product) Description() string {
return p.description
}
// Price returns the price of the product.
func (p *product) Price() float64 {
return p.price
}
// Category returns the category of the product.
func (p *product) Category() string {
return p.category
}
// Display prints information about the product.
func (p *product) Display() {
fmt.Println("Name: ", p.name)
fmt.Println("Description: ", p.description)
fmt.Println("Price: ", p.price)
fmt.Println("Specifications:")
for specTitle, specInfo := range p.specifications {
fmt.Println(specTitle)
for _, specDesc := range specInfo {
fmt.Println(specDesc)
}
}
}
// Images returns a list of image urls of the product.
func (p *product) Images() []string {
return p.images
}
// IsValid checks if a product is valid and returns true if it is valid.
func (p *product) IsValid() bool {
return p != nil && p.name != "" && p.productType != "" && p.description != "" &&
p.price > 0 && len(p.images) != 0 && len(p.specifications) != 0
}
// CreatedAt returns when this product was created.
func (p *product) CreatedAt() *time.Time {
return p.createdAt
}
// LastUpdated returns the date this product was last updated.
func (p *product) LastUpdated() *time.Time {
return p.lastUpdated
}
// car is a store product, embeddeds the product struct and re-implements
// several methods defined by the Product interface.
type car struct {
*product
color string
make string
model string
year string
}
// Display implements part of the Product interface for car.
func (c *car) Display() {
fmt.Println("Name: ", c.DisplayName())
fmt.Println("Make and Model: ", c.make, c.model)
fmt.Println("Specifications:")
for specTitle, specInfo := range c.specifications {
fmt.Println(specTitle)
for _, specDesc := range specInfo {
fmt.Println(specDesc)
}
}
}
// IsValid implements part of the product interface for car.
func (c *car) IsValid() bool {
return c.product != nil && c.product.IsValid() && c.make != "" &&
c.model != "" && c.color != ""
}
The code snippet above shows the different go types I utilised in my solution with comments describing their use. In my most recent version, I removed or renamed several methods of the product interface and added new ones. For example, I added the `Product()` method that returns a pointer to the embedded product struct which contains the basic product information.
This allows the `store` to manipulate the product fields without type casting.
To sum this part up: I created the "car" struct with relevant attributes such as make, model, and year. I also included a method to display the attributes of the "car" struct. I also created the "product" struct, which included attributes for the product name, ID, and price and added a method to display the attributes of the product struct. I then promoted the attributes of the "product" struct which implements our "Product" interface to the "car" struct, since a car is a type of product that can be sold in the store and the "car" struct will only have to implement methods of the "Product" interface it wants to behave differently.
I also created the "order" struct with relevant information for a typical order and also created other relevant types like "productID" and "orderID" with some useful methods.
Logic in my store.go file:
package main
import (
"crypto/rand"
"errors"
"fmt"
"log"
"sync"
"time"
)
// store is the keeps track of all the existing and sold products.
type store struct {
name string
mtx sync.RWMutex
products map[productID]Product
processedOrders map[orderID]*order
}
// newStore creates a new store.
func newStore(name string) *store {
store := &store{
name: name,
products: make(map[productID]Product),
processedOrders: make(map[orderID]*order),
}
return store
}
// addProducts adds new product(s) and returns an array of product IDs.
func (s *store) addProducts(products ...Product) ([]productID, error) {
s.mtx.Lock()
defer s.mtx.Unlock()
if len(products) == 0 {
return nil, errors.New("provide one or more products")
}
// Validate products.
for _, product := range products {
if product == nil {
return nil, errors.New("invalid product")
}
if !product.IsValid() {
return nil, fmt.Errorf("product with ID %s is not valid or missing required fields", product.ID().String())
}
}
now := time.Now()
productIDs := make([]productID, len(products))
for i, p := range products {
product := p.Product()
// Generate a new ID for this product.
s.generateProductID(product)
// Set essential product dates.
product.createdAt = &now
product.lastUpdated = &now
// Add product to store products map and also add the product ID to
// return to callers.
productID := p.ID()
s.products[productID] = p
productIDs[i] = productID
}
return productIDs, nil
}
// sellProduct sells one or more product to a buyer and returns the order ID.
func (s *store) sellProduct(order *order) (orderID, error) {
if order == nil || order.shippingAddress == "" || order.amountPaid <= 0 || order.name == "" || len(order.products) == 0 {
return zeroOrderID, errors.New("order is missing required fields")
}
var totalProductCost float64
for _, p := range order.products {
if p == nil {
return zeroOrderID, errors.New("invalid product")
}
if _, ok := s.products[p.ID()]; !ok {
return zeroOrderID, fmt.Errorf("product with ID %s does not exist", p.ID().String())
}
if !p.IsValid() {
return zeroOrderID, fmt.Errorf("product with ID(%s) is not valid", p.ID())
}
totalProductCost += p.Price()
}
// Check if buyer paid enough.
if order.amountPaid < totalProductCost {
return zeroOrderID, fmt.Errorf("order amount paid is not enough, need %f but paid %f", totalProductCost, order.amountPaid)
}
s.mtx.Lock()
for _, p := range order.products {
delete(s.products, p.ID())
}
// Generate new order ID.
s.generateOrderID(order)
s.processedOrders[order.id] = order
s.mtx.Unlock()
return order.id, nil
}
// product returns a single product if it is found.
func (s *store) product(ID productID) Product {
s.mtx.RLock()
defer s.mtx.RUnlock()
product, ok := s.products[ID]
if !ok {
return nil
}
return product
}
// availableProducts returns the available products matching the provided
// product type, and their total cost if they are in stock. If no product type
// is specified, all the products in the store, and their prices are returned.
func (s *store) availableProducts(productType string) ([]Product, float64) {
s.mtx.RLock()
defer s.mtx.RUnlock()
var products []Product
var totalCost float64
if productType == "" {
for _, product := range s.products {
products = append(products, product)
totalCost += product.Price()
}
return products, totalCost
}
for _, product := range s.products {
if product.Type() == productType {
products = append(products, product)
totalCost += product.Price()
}
}
return products, totalCost
}
// soldProducts returns the sold products matching the provided product type,
// and their total cost. If no product type is specified, all the sold products
// in the store, and their prices are returned.
func (s *store) soldProducts(productType string) ([]Product, float64) {
s.mtx.RLock()
defer s.mtx.RUnlock()
var products []Product
var totalCost float64
if productType == "" {
for _, orders := range s.processedOrders {
for _, product := range orders.products {
products = append(products, product)
totalCost += product.Price()
}
}
return products, totalCost
}
for _, orders := range s.processedOrders {
for _, product := range orders.products {
if product.Type() == productType {
products = append(products, product)
totalCost += product.Price()
}
}
}
return products, totalCost
}
// orders returns a list of processed orders.
func (s *store) orders() ([]*order, float64) {
s.mtx.RLock()
defer s.mtx.RUnlock()
var orders []*order
var totalPaid float64
for _, order := range s.processedOrders {
orders = append(orders, order)
totalPaid += order.amountPaid
}
return orders, totalPaid
}
// deleteProducts removes one or more available product from the store and
// return the number of products deleted. It will be a no-op if product does not
// exist.
func (s *store) deleteProducts(productIDs ...productID) (int, error) {
if len(productIDs) == 0 {
return 0, errors.New("provide one or more product IDs")
}
s.mtx.Lock()
defer s.mtx.Unlock()
var deleted int
for _, productID := range productIDs {
if _, ok := s.products[productID]; ok {
delete(s.products, productID)
deleted++
}
}
return deleted, nil
}
// inStock checks if the specified product type is in this store and
// in stock.
func (s *store) inStock(productType string) bool {
s.mtx.RLock()
defer s.mtx.RUnlock()
for _, product := range s.products {
if product.Type() == productType {
return true
}
}
return false
}
// generateProductID generates a random ID for a product.
func (s *store) generateProductID(product *product) {
_, err := rand.Read(product.id[:])
if err != nil {
log.Println(err)
}
}
// generateOrderID generates a random ID for an order.
func (s *store) generateOrderID(product *order) {
_, err := rand.Read(product.id[:])
if err != nil {
log.Println(err)
}
}
The above code snippet shows the essential logic of the solution for the car-selling store. Major refactoring here includes removing some unnecessary restrictions, bug fixes(where we weren't checking if the amount paid by a buyer was enough to purchase our product!) and ensuring the store assigns important fields to the product like ID and date fields.
I created the "store" struct which holds a map of productIDs and their corresponding product. Notice how they all exist in the same map because we are using the "Product" interface which does not care what underlying type the product is as long as it implements all the methods specified by the "Product" interface.
Note: It is good practice to use "mutex lock" to protect struct fields like maps from parallel modification just like our store does.
The last part is in my main.go:
package main
import (
"fmt"
"os"
)
func main() {
autoShopSimulation()
}
/*
autoShopSimulation runs a simulation of the various functionalities implemented
by Store. Simulation of expected features and requirements are not in any
specific order.
User Story:
Build an auto shop inventory with the following features.
Requirements:
1. Shop owner needs to see the number of cars that are left to be sold
2. Shop owner needs to see the sum of the prices of the cars left
3. Shop owner needs to see the number of cars he has sold
4. Shop owner needs to see the sum total of the prices of cars sold
5. Shop owner needs to see a list of orders that for the sales made
The Store class should have attributes like:
1. Adding an Item to the store
2. Number of products in the store that are still up for sale
3. Listing all product items in the store
4. Sell an item
5. Show a list of sold items and the total price
*/
func autoShopSimulation() {
// These are the supported product type for our Auto-Shop.
productTypeCar, productTypeCarAccessory := "Car", "Car Accessory"
// newStore creates a store that can sell different products. All product
// prices in this store are denominated in the Nigerian Naira.
autoShop := newStore("Auto Shop")
item1 := &car{
product: &product{
name: "Ford Ecosport",
price: 5000000,
productType: productTypeCar,
category: "Used Cars",
description: "The EcoSport is easy to drive and spacious inside. The 1.0-litre petrol engine is a popular choice because of its efficiency.",
images: []string{"https://uks-cdn.pinewooddms.com/b04b90f8-2e99-463d-a023-7e3c771fb388/vehicles/1935a96a-3bb8-485e-affc-132707e733c1.jpg?", "https://uks-cdn.pinewooddms.com/b04b90f8-2e99-463d-a023-7e3c771fb388/vehicles/4cb99337-5c1b-4f0e-9bb7-3683f23520de.jpg?"},
specifications: map[string][]string{
"Key Features": {"Bluetooth", "Climate Control", "Air Conditioning", "Ask for a Test Drive Today", "24 Month Guarantee Available", "2 x Keys with car"},
"Engine": {"Auto", "Petrol"},
},
},
color: "yellow",
make: "Ford",
model: "1.5 Zetec 5dr",
year: "2016",
}
item2 := &car{
product: &product{
name: "Honda HR-V SPORT",
price: 7000000,
productType: productTypeCar,
category: "Used Cars",
description: "The Honda HR-V SPORT easy to drive and spacious inside. The automatic engine is a popular choice because of its efficiency.",
images: []string{"https://content.homenetiol.com/698/2163991/1920x1080/8ac0270d04d344b1ad58ae18e01c4c88.jpg", "https://content.homenetiol.com/698/2163991/1920x1080/ae3d1b14b4614451938dd3703a18222a.jpg"},
specifications: map[string][]string{
"Key Features": {"Bluetooth", "Cruise Control", "4 Doors", "Rear Defroster", "Climate Control", "Air Conditioning", "Ask for a Test Drive Today", "24 Month Guarantee Available", "2 x Keys with car"},
"Engine": {"Auto", "Petrol", "4 Cylinders 1.8L"},
},
},
color: "black",
make: "Honda",
model: "4 Cylinders 1.8L",
year: "2018",
}
item3 := &product{
name: "Toyota Shadow Logo Led Light (For 4 Doors)",
price: 14000,
productType: productTypeCarAccessory,
category: "Led Lights",
description: "TOYOTA LED HOLOGRAM SAFETY LIGHTS(free batteries included): Stay safe at night when stepping out of your cars in poorly lit areas with our classy, elegant light emitting diode car door lights.",
images: []string{"https://ng.jumia.is/unsafe/fit-in/500x500/filters:fill(white)/product/74/552546/1.jpg?6525"},
specifications: map[string][]string{
"Key Features": {"Toyota LED Hologram Safety Lights, Free batteries included"},
},
}
// Add different supported products to the store.
// Store Feature 1.
productIDs, err := autoShop.addProducts(item1, item2, item3)
if err != nil {
fmt.Println(err)
os.Exit(1)
}
for _, id := range productIDs {
fmt.Printf("Successfully added product with ID(%s) to %s\n", id, autoShop.name)
}
// Store Feature 2 and 3.
// Retrieve information for all products in the store.
allAvailableProducts, totalCost := autoShop.availableProducts("")
fmt.Printf("%s has %d products available that cost a total of %.2f NGN\n", autoShop.name, len(allAvailableProducts), totalCost)
// Retrieve information for a specific product kind in the store.
allAvailableProducts, totalCost = autoShop.availableProducts(productTypeCar)
fmt.Printf("%s has %d %s's available that cost a total of %.2f NGN\n", autoShop.name, len(allAvailableProducts), productTypeCar, totalCost)
// Store feature 4.
order := &order{
name: "Philemon",
amountPaid: item1.price + item3.price,
shippingAddress: "No 21 Alt_School Africa street, Banana Island, Lagos",
products: []Product{item1, item3},
}
orderID, err := autoShop.sellProduct(order)
if err != nil {
fmt.Println(err)
os.Exit(1)
}
fmt.Printf("%s has processed order with ID(%s) successfully\n", autoShop.name, orderID)
// Store Feature 5.
allSoldProducts, totalCost := autoShop.soldProducts("")
fmt.Printf("%s has sold a total of %d products for %.2f NGN\n", autoShop.name, len(allSoldProducts), totalCost)
// Requirement 3 and 4.
allSoldCars, totalCost := autoShop.soldProducts(productTypeCar)
fmt.Printf("%s has sold %d %s for %.2f NGN\n", autoShop.name, len(allSoldCars), productTypeCar, totalCost)
// Requirement 1 and 2.
allAvailableCars, totalCost := autoShop.availableProducts(productTypeCar)
fmt.Printf("%s has %d %s available that cost a total of %.2f NGN\n", autoShop.name, len(allAvailableCars), productTypeCar, totalCost)
// Shop feature 5 and Requirement 5.
processedOrders, totalPaid := autoShop.orders()
fmt.Printf("%s has processed %d orders totalling %2.f NGN\n", autoShop.name, len(processedOrders), totalPaid)
// Check that products are in stock.
inStock := autoShop.inStock(productTypeCar)
fmt.Printf("%s has a %s in stock: %v\n", autoShop.name, productTypeCar, inStock)
inStock = autoShop.inStock(productTypeCarAccessory)
fmt.Printf("%s has a %s in stock: %v\n", autoShop.name, productTypeCarAccessory, inStock)
// Check product availability.
product := autoShop.product(item1.id)
fmt.Printf("Sold product with id %s is available: %v\n", item1.id, product != nil)
// Delete products from store.
deleted, err := autoShop.deleteProducts(productIDs...)
if err != nil {
fmt.Println(err)
os.Exit(1)
}
fmt.Printf("Deleted %d product(s) from %s\n", deleted, autoShop.name)
}
This part had little refactoring, improved print output added new instances to the stimulation and moved the simulation logic into a separate function.
Want to run this? Head over to the GitHub repo here: https://github.com/ukane-philemon/gstore
Lessons Learned:
Code review and refactoring are invaluable. Do them from time to time but not prematurely.
Pay keen attention to clients' product specifications but don't be reluctant to let them know what works and what does not.
If you need to learn more about Go, visit: