Refactoring: Choosing abstractions
How do I pick the right abstractions? Are there rules that can be followed?
Let’s look at an example, and see if we can apply some rules to make code cleaner.
Example
Assume you are a dev working on a Web3 project where users can send tokens as gifts to one another.
Given a function:
import { makeError, getTokenById, getWalletByQuery, findUserByQuery, addTokenEscrow, giftNewUserToken } from "./utils";
const giftToken = (jwtUser, values, build_number, token_id) => {
const { _id, type, fname, name } = jwtUser;
const { email, message, username } = values;
let amount = Number(values.amount);
const token = await getTokenById(token_id);
if (!token) {
throw makeError("NOT_FOUND_ERROR", "Token not found");
}
if (!build_number) {
amount /= token.price.sell;
}
const wallet = await getWalletByQuery({ user: _id, token: token._id });
if (!wallet) {
throw makeError("PRECONDITION_FAILED_ERROR", "You don't have this token.");
}
if (Number(wallet.balance) < amount) {
throw makeError("PRECONDITION_FAILED_ERROR", "Insufficient token balance");
}
const user = email
? await findUserByQuery({ email })
: await findUserByQuery({ username });
const to = user._id;
const from = _id;
if (String(from) === String(to)) {
throw makeError("FORBIDDEN_ERROR", "You cannot gift tokens to yourself");
}
// Check receiver access if it's a private token
if (token.type === TokenTypes.PRIVATE) {
throw makeError("FORBIDDEN_ERROR", "Private tokens cannot be gifted.");
}
const extraData = {
initiator: "giftTokens",
email: user.email,
message,
token: token_id,
sender_model: type.toLowerCase()
};
const [gift] = await Promise.all([
addTokenEscrow(token_id, amount, from, false, extraData)
]);
return gift;
}
😰
This is a huge function that does many things. But it’s not uncommon to find functions like this in the wild.
It is our task to refactor this function, clean it up, and make it better.
Step 1: Define what the function does
The first thing I like to do when refactoring a function, is figure out exactly what the function does. A well written pseudocode for the function will help us with cleaning it up.
const giftToken = (jwtUser, values, build_number, token_id) => {
/**
* input:
* - a user, who sends the token
* - values for a receiver user
* - token id for the token to be gifted
*
* steps:
* - get the token info by its id
* - if token info does not exist
* - throw an error
* - divide the values amount by the token's sell price if `build_numer` is not defined
* - get the user's wallet info for the token using the sender's id
* - if the wallet does not exist
* - throw an error
* - if the wallet balance is < amount
* - throw an error
* - get the receiver user by either its email or username
* - if sender and receiver have the same id
* - throw an error
* - if token type is private
* - throw an error
* - create escrow transaction for the token gift
*/
}
From the steps, we can see that our function is concerned with details like:
getTokenById
updating amount if !build_number
wallets
wallet balances
verifying sender <> receiver
verifying token type is !private
before actually doing the thing it’s supposed to do, which is creating the transaction in escrow
.
Step 2: Refine what the function does
For every step in the pseudocode above, we should ask ourselves:
Does this really need to be here?
Can this be part of a previous step?
For instance, the step that updates the amount if !build_number
is obscure in what it does. What exactly is a build_number
and what does it have to do with gifting tokens?
Does this really need to be here?
No!
We can have the amount
, resolved in a previous step, and explicitly passed into this function as an argument.
const giftToken = (jwtUser, values, amount, token_id);
For another instance, the wallets
and wallet balances
steps are not referenced anywhere else, and only exist to confirm that the user has the amount of the token they want to gift. They can be combined into assertUserHasTheTokenAmountTheyWantToGift
.
So our steps become:
getTokenById
assertUserHasTheTokenAmountTheyWantToGift
verifying sender <> receiver
verifying token type is !private
create escrow transaction
The verify
steps seem to be fine since they don’t reference details that are external to gifting tokens.
The create escrow transaction
steps are external to gifting tokens, so they can be abstracted.
Step 3: Summarize into an explicit theory
An explicit theory is the simplest language that can be used to describe what a function does.
In our case, our explicit theory can be written as:
A token is gifted from a
sender
to areceiver
, only if thesender
has a token balance that is greater than or equal to theamount
of the token they are attempting to send. When all verifications are successful, anescrowTransaction
is created on behalf of thesender
.
Using our explicit theory, we can then come up with our abstractions:
const giftToken = async (sender, receiver, tokenId, amount,
{
getTokenById,
assertUserHasTheTokenAmountTheyWantToGift,
createEscrowTransaction
}) => {
const token = await getTokenById(tokenId);
await assertUserHasTheTokenAmountTheyWantToGift(sender, token, amount);
verifyGiftPartiesAreDistinct(sender, receiver);
verifyTokenTypeIsNotPrivate(token);
return createEscrowTransaction(sender, token, amount);
};
function verifyGiftPartiesAreDistinct(sender, receiver) {
if (sender._id === receiver._id) {
throw makeError(...);
}
};
function verifyTokenTypeIsNotPrivate(token) {
if (token.type === TokenTypes.PRIVATE) {
throw makeError(...);
}
};
const getTokenById = async tokenId => {
// retrieve token by id from a repository
};
const assertUserHasTheTokenAmountTheyWantToGift = async (sender, token, amount) => {
// get wallet by sender._id and token._id
// if !wallet
// throw error
// if wallet.balance < amount
// throw error
};
const createEscrowTransaction = async (sender, token, amount) => {
// create escrow trx
};
You will notice that the names of the variables have changed from jwtUser — > sender
and this change is informed by our explicit theory, which is formed by our understanding of what the function should do.