Engineering > Style Guide

Procuret Engineering Style Guide

When writing code in Procuret, you should follow the following guidelines. Like a style guide for a publication, they are not hard-and-fast rules. Instead they are rules for the "general case", ones that you should generally follow unless there is a compelling reason to do otherwise.

No lines longer than 79 characters

Self explanatory. Put a ruler in your text editor. Breaking this rule is very unlikely to be acceptable in imperative code. You will probably need to break it occasionally while writing declarative code (e.g. HTML or SQL), but you should assiduously avoid doing so.

All datastructures must have types

No raw maps or collections. No naked value types (e.g. strings, integers) unless their meaning is explicit in context.


# Unacceptable Pythone example
def create_account(name: str) -> Dict[str, Any]:
    account_id = RandomNumber()
    return {
        'name': name,
        'account_id': account_id
    }

account = create_account(name='Assets')

Outside the call site context and without the create_account(...) definition handy, the account Dict is worse than useless, it is dangerous.


# Acceptable Python Example
class Account:
    def __init__(self, name: str) -> None:
        self._account_id = RandomNumber()
        self._name = name
        return

account = Account(name='Assets')

We now have assurance about what account is. Access to data is now controlled by the API defined in the Account type.

One type definition per file

Got a file called Account.swift? That file should contain, at most, one single type definition. For example, struct Account {...}. Rare exceptions might be small value type definitions that are only applicable inside that file.

Define all types statically

Every single function, method, and constructor signature must explicitly define static type requirements. This applies even to languages that do not perform their own static type checking, such as Python. The type definition is for you and your fellow programmers, not the machine.

Where a language does not have the ability to record static type definitions (e.g. JavaScript), add them with comments.


# Unacceptable Python example
def do_thing(parameter_1):
    # Do work...

# Acceptable Python example
def do_thing(parameter_1: int) -> float:
    # Do work...


// Unacceptable JavaScript example
function doThing(firstParameter, secondParameter) {
    // Do work...
}

// Acceptable JavaScript example
function doThing(
    firstParameter,   // integer number
    secondParameter   // string
) { // Returns floating point number
    // Do work
}

Even though the concept of an "integer", "float" or "string" is at best amorphous in JavaScript, this signature gives useful information to future-you and your colleagues.

Check security and money types at runtime

Where a compiler cannot provide you with absolute assurance of type safety, and a type has a bearing on code that will result in...

Authentication or authorisation of a user, or
Computation of a monetary amount

... Then you must perform dynamic type checking. The "downside" risk in such scenarios is very high. Lean on the machine to give you absolute assurance that you are working with what you think you are working with.


# Unacceptable Python example
def grant_read_to(self, user: User) -> bool:
    return user.read_permission == True

# Acceptable Python example
def grant_read_to(self, user: User) -> bool:
    assert isinstance(user, User)
    return user.read_permission == True


// Unacceptable JavaScript example
function computeBalanceOwed(
    transactions      // Array<Transaction<
) {  // Returns decimal number
    return transactions.reduce((total, currentValue) => {
        return total + currentValue;
    });
}

There is no acceptable variant of this JavaScript code. JavaScript must never be used for money or authentication/authorisation work in Procuret.

Name all arguments

Wherever the language allows, explicitly name your arguments. The only exception is if your function takes a single argument.


// Unacceptable Swift example
func doThing(_ parameter1: Int, _ parameter2: String) -> Int {
    // do work
}

doThing(7, 'hello')

// Acceptable Swift example
func doThing(parameter1: Int, parameter2: String) -> Void {
    // do work
}

doThing(parameter1: 7, parameter2: 'hello')


# Unacceptable Python example
def do_thing(parameter1: int, parameter2: str) -> float:
    # Do work

done_thing = do_thing(42, 'hello')

# Acceptable Python example, single parameter
def do_thing(parameter: int) -> float:
    # Do work

done_thing = do_thing(42)

No abbreviations

Acryonyms or foreshortened words are not to be used in Procuret code. A rare exception might be for a universal acronym for an enumerated value, such as an ISO 4217 currency code.

Names should be natural English expressions, within the style conventions of the given language.


# Unacceptable Python example
desc = 'Foreshortened description'
cac = 2   # Indecipherable acronym

# Acceptable Python example
description = 'Full natural English word'
customer_account_code = 1
aud = Currency(iso_4217_code='aud') # Acceptable acronym

The standard library should be the only third party dependency

A third party dependency should be your last resort. You can delegate authority, but you cannot delegate responsibility. Any third party code you add to Procuret becomes our responsibility. Is that worth saving you from writing a small JavaScript date class? Almost certainly not.

Obviously, Procuret has a great many third party dependencies. These dependencies generally handle very low level tasks like database connections, or provide clean type definitions for third party services. There are very good reasons to introduce third party dependencies. However, your default stance should always be to lean on your own types and the standard library.

This rule applies most strictly to both client code, running in a browser or natively, and our own libraries.

Retain backwards compatibility

Bend over yourself backwards to retain backwards comptability in all our APIs. This applies equally to intra-application APIs like public class definitions, and our our own libraries.

Do work declaratively

Declare what you want done, rather than how you want it done. Most simply, this means leaning on declarative language features. For example, List comprehensions in Python:


# Example of imperative "how" code in Python

def parse_many(things: List[Dict[str, Any]]) -> List[str]:

    matching_things: List[Any] = []
    for thing in things:
        if thing['foo'] == 'bar':
            matching_things.append(thing['foo'])
        continue

    return matching_things

# Example of declarative "what" code in Python

def parse_many(things: List[Dict[str, Any]]) -> List[str]:
    return [thing for thing in things if thing['foo'] == 'bar']

More broadly, it means defining declarative interfaces for your types. For example:


// Example of imperative "how" code in Swift
if !account.authorisedUserIds.contains(agent.agentId) {
    throw NotAuthorisedError
}

// Example of declarative "what" code in Swift
if !account.grantsReadTo(agent) {
    throw NotAuthorisedError
}

The above example implies that the "how" has been moved into a Account.grantsReadTo method with signature (_ agent: AgentXYZ) -> bool.

No inheritance

Compose your objects out of discrete independent types, rather then inheriting.


# Human definition shared between both unacceptable and acceptable examples

class Human:
    def __init__(self, name: str) -> None:
        self._name = name
        return
    
# Unacceptable Python code

class User(Human):
    def __init__(self, name: str, user_id: int) -> None:
        self._user_id = user_id
        return super().__init__(name=name)

user = User(name='Charlotte', user_id=420)

# Acceptable Python code

class User:
    def __init__(self, user_id: int, human: Human) -> None:
        self._user_id = user_id
        self._human = human
        return

Note that this rule does not apply to "protocols" or "abstract" classes - Generally, if you are inheriting from a type that does not have an initialiser / constructor, you are doing the right thing. For example, the following pattern is actively encouraged:



class PubliclyIdentified:
    public_id: str = NotImplemented

class Human(PubliclyIdentified):
    def __init__(self, name: str, public_id: str) -> None:
        self._name = name
        self._public_id = public_id
        return

    public_id = property(lambda s: s._public_id)

Match platform code formatting conventions

Wherever possible, match an existing platform code aesthetic convention. For example, in Python follow PEP 8 conventions. In JavaScript follow MDN's JavaScript guidelines.

No global variables

Your code must not use any variables defined outside local scope.


# Unacceptable JavaScript code
let globalVariable = 1;

function addSomething() { // Returns integer
    return globalVariable + 1;
}

# Acceptable JavaScript code
function addOneToAnother(
    another // Integer
) { // Returns integer
    return another + 1;
}

Global constants are ok!


# Acceptable JavaScript code
const SPEED_OF_LIGHT = 299792458;  // metres per second

function proportionOfLightSpeed(
    speed  // Number in metres per second
) {  // Returns float
    return speed / SPEED_OF_LIGHT;
}

No state

In practice, state is inevitable, and software always has state. Our objective is to keep as little state as possible. For example, consider the following type:


# Unacceptable Python code
class Human:

    LOOKUP_NAME = 'select name from humans where id = {}'
    UPDATE_NAME = 'update humans set name = {} where id = {}'

    def __init__(self, human_id: int, database: Database) -> None:
        self._id = human_id
        self._name = database.execute(self.LOOKUP_NAME, human_id)
        self._database = database
        return

    def update(self, new_name: str) -> None:
        self._database.execute(self.UPDATE_NAME, new_name, human_id)
        return

An instance of a Human is bound tightly to an instance of Database. The state of the Database instance affects the Human. And what happens if there is no Human with the specified ID? Should the caller expect one to be created? Will Human have some semi-initialised state where properties maybe do not exist? Do not write code like this.

Instead, consider...


# Acceptable Python code
H = TypeVar('H', bound='Human')


class Human:

    LOOKUP_NAME = 'select name from humans where id = {}'
    UPDATE_NAME = 'update humans set name = {} where id = {}'

    def __init__(self, human_id: int, name: str) -> None:
        self._human_id = human_id
        self._name = name
        return

    def update(self, database: Database, new_name: str) -> H:
        database.execute(self.UPDATE_NAME, new_name, self._human_id)
        return self.retrieve(database=database, human_id=self._human_id)

    @classmethod
    def retrieve(
        cls: Type[T],
        database: Database,
        human_id: int
    ) -> Optional[H]:
        name = database.execute(cls.LOOKUP_NAME, human_id)
        if name is None:
            return None
        return cls(human_id=human_id, name=name)

The state of the database is separated from the state of the human, and there is absolute certainty that an initialised Human has all its defined properties.

All properties should be immutable

Ultimately, the Procuret database holds the authoritative true state of all data in Procuret. Types you define can only change that state by interacting with the database. Do not allow your types to be changed in such a way that would imply that they have changed Procuret's state. For example, consider the following, using the Human definition from above, but with a changed constructor:


# Human definition as above, but for the following change:

class Human:
    # as above
    def __init__(self, human_id: int, name: str) -> None:
        self._human_id = human_id
        self.name = name
        return
    # as above

# assume we have a Human in the database with ID `420`
# assume database is defined elsewhere
human = Human.retrieve(human_id=420, database=database)
assert isinstance(human, Human)
human.name = 'New Name'

This instance of Human now has name "New Name. However, to the rest of the Procuret system, that human still has its original name. We have created new state by allowing mutation of Humanproperties. Make all properties immutable, using whatever tools are available in the language. For example:


# Acceptable Python
class Human:
    def __init__(self, human_id: int, name: str) -> None:
        self._human_id = human_id
        self._name = name
        return

    human_id = property(lambda s: s._human_id)
    name = property(lambda s: s._name)


# Acceptable JavaScript
class Human {
    constructor(
        humanId,   // Integer
        name       // String
    ) {
        this._humanId = humanId;
        this._name = name;
        return;
    }

    get humanId() { return this._humanId; }
    get name() { return this._name; }
}


#Acceptable Swift
struct Human {
    let humanId: Int
    let name: String
}

Note that of the three above examples, only the Swift example is immutable from the perspective of memory. However, our objective is not to be immutable from the perspective of memory (though, that is a nice bonus in the Swift case), our objective is to be immutable from th perspective of ourselves and our colleagues.

In the Swift example, we will get a compiler error if we try to mutate .name or .humanId. In Python and JavaScript, we will get runtime errors. Both achieve the intended outcome: Stop us from blowing our legs off.

Introduce all dependencies at the call site

There is a million-dollar name for this $2 concept: "Dependency Injection". It can be reduced to "don't use any variables not created in current scope or introduced through the function signature." For example:


# Unacceptable Python code
from somewhere import database

def put_value_in_database(value: int) -> None:
    database.execute(some_query, value)
    return

The database variable was introduced via a module import. Bad!


# Acceptable Python code
def put_value_in_database(value: int, database: Database) -> None:
    database.execute(some_query, value)
    return

By introducing all dependencies when a function is called, we reduce the amount of state we have to reason about. Code becomes much easier to test. And types can be re-used in wildly varying contexts.

Throw early and throw often

As soon as your code has the opportunity to gain assurance over the data it is dealing with, take that opportunity. If something is amiss, throw immediately.


# Unacceptable JavaScript code
class InputForm {
    constructor(
        container  // HTML Element,
    ) {
        this._inputField = new InputField(
            container.getElementsByTagName('input')[0]
        );
        
        return;
    }
}

const form = new InputForm(
    document.getElement
)

In the above example, container is effectively optional when it enters the InputForm constructor. If it does not hold a value, we will only find out when we call .getElementsByTagName(). In this simple example, that is probably fine. In practical code, the effect of bad data penetrating deep into the stack is pernicious. Catch it as far up the stack as possible.


# Acceptable Javascript code
class InputForm {
    constructor(
        container  // HTML Element,
    ) {
        if (!container) { throw Error('Missing container!'); }

        this._inputField = new InputField(
            container.getElementsByTagName('input')[0]
        );
        
        return;
    }
}

Define program control explicitly

Ensure all code paths are explicitly handled.


# Unacceptable Python code
def find_thing(things: List[str]) -> str:
    for thing in things:
        if thing == 'constant':
            return thing

# Acceptable Python code
def find_thing(things: List[str]) -> str:
    for thing in things:
        if thing == 'constant':
            return thing
        continue
    raise RuntimeError('thing not found')

Conclusion

Some say "rules are meant to be broken". Well, these rules are meant to be followed. By sticking to them you will keep the Procuret codebase more readable, maintainable, performant, and reliable. You will need to break them at some point. Take care to ensure that you are only doing so for very good reasons.