tilteng / api-style-guide

Best practices for API development at Tilt.
1 stars 0 forks source link

Tilt API Style Guide

This style guide is a general style guide for our API perl coding practices and standards. For more specific current best practices on specific features or areas of the codebase, see the corresponding .md file in this repo for more info on that topic.

Table of Contents

General Perl

Follow the jQuery style guide

The following rules take precedence over the jQuery guide:

Always use tilt::core

tilt::core gives us a robust set of baseline Perl modules, pragmas, and features, and we should always be importing this module first.

use tilt::core;

However, if another module that alters the current file with a custom DSL (Domain-specific language) is loaded first, such as Dancer or Moose, then that module should take precedence.

use Dancer;
use tilt::core;

...

dance;

Favor strict mode

It is better to import tilt::core with the :strict_fp flag instead of the default "lax" mode, and this utilizes Function::Parameters :strict under the hood (enables argument checks so that an improper number of arguments will throw an error). We are still in the process of migrating our existing modules from "lax" to "strict", but we should follow this pattern for all greenfield development.

use tilt::core qw(:strict_fp);

Favor newer Tilt::* imports

It is clearer to import the newer Tilt::Error and Tilt::Util modules over Crowdtilt::Internal::Error and the Crowdtilt::Internal::Util::* namespace.

use Tilt::Error;
use Tilt::Util qw(BankAccount Campaign User);

Use structured exception handling

We should utilize try/catch/finally over Perl's magical $@ exception handling. It is also best practice to catch specific, typed exception classes before the stringified "catch-all" exception.

try {
    my $result = 1/0;
} catch (CustomExceptionClass $exc) {
    print "Exception: $exc";
    die $exc;
} catch ($err) {
    print "Something else went wrong: $err";
} finally {
    do_cleanup();
}

Avoid the Smartmatch operator

Smart matching ~~ is considered deprecated and carries with it a number of issues and inconsistencies, so use the |in| operator instead to check whether an element is present inside of an ArrayRef.

$match = 5 |in| [ 5, 6, 7, 8 ];

Avoid the Enterprise operator

Instead of using the syntactically noisy Enterprise operator ()x!! to conditionally construct our HashRefs, we have the clearer maybe and provided that are in more familiar English.

my $person = {
    maybe name => $name,
    maybe age  => $age,
    provided $inches > 200, height => $inches,
    provided $pounds < 200, weight => $pounds,
};

Avoid unnecessary line noise

Avoid introducing write-only statements, or line noise, where the code looks like spurious characters from signal noise in the communication line. Omitting parenthesis where they are not required, and carefully utilizing whitespace can enhance readability considerably.

# Good
push @$it, $real, $good;
map { throw_away bag_up $_ } grep { /poop/ } map { $_->walk } $dog, $cat;

# Bad
push( @{$it}, ($real, $good) );
map(throw_away(bag_up($_)), (grep($_ =~ /poop/, (map ($_->walk(), ($dog, $cat))))));

Keep it under 80 characters

Do not exceed 80 characters per line of code. There are exceptions, but generally if one cannot fit everything onto one line, it is best practice to break up the long statement into multiple lines.

return $self->contributions->search({
    status  => [qw(status_one status_two status_three)],
    user_id => $self->user_id,
})->get_column('netamount')->sum // 0;

Follow snake_case convention

Make sure you are naming methods and variables using the snake_case naming convention and not camelCase, TitleCase, or some other naming convention.

fun calculate_gross_profit_margin(Int $gross_profit, Int $total_revenue) {
    return $gross_profit / $total_revenue;
}

Vertically align hashes

For our Hash and HashRef declerations, we indent key/value pairs so that they are aligned on the Fat Comma => operator to enhance readability.

my $vars = {
    api_org             => $campaign->org,
    from_email          => _cdata_decode($from->email),
    body                => _cdata_decode($body),
    subject             => _cdata_decode($subject),
    admin_name          => _cdata_decode($admin->full_name),
    campaign_short_link => $campaign->get_short_link,
    campaign_title      => _cdata_decode($campaign->title),
    reply_email         => $admin->email,
};

Prefer vertically aligned assignments

This one is not a hard requirement, but it can enhance readability for long stretches of Assignment = operators on variables.

my $from    = smart_rset('User')->on_crowdtilt->find($msg->{from_id});
my $to      = smart_rset('User')->on_crowdtilt->find($msg->{to_id});
my $body    = $msg->{body};
my $subject = $msg->{subject};
my $locale  = locale_from_user_country_code $to->country_code;

Function Signatures

tilt::core makes use of Function::Parameters under the hood, and our codebase tries to make use of Function::Parameters as much as possible. It gives us (runtime) type checking, and cleaner function signatures with optionally typed parameter lists.

With Function::Parameters we use the fun and method keywords for functions and methods, respectively. The method keyword behaves the same as fun, except that it automatically populates the $self variable with the first argument to the function (making it a method).

Here are some simple examples showing the standard Perl way, vs the Function::Parameters way:

# Perl way
sub foo {
    my ($bar, $baz) = @_;
    ...
}
# F::P way
fun foo($bar, $baz) {
    ...
}

# Perl
sub foo {
    my (%args) = @_;
    my $keyword = $args{keyword};
    ...
}
# F::P (The ':' denotes a hash key value)
fun foo (:$keyword) {
    ...
}

# Setting a Type with F::P
# Also *requires* $string to be passed
fun foo (Str $string) { ... }

# Specifying a Type + making field optional
fun foo (Maybe[Str] $string = undef) { ... }

# Specifying a Type + setting a default value
fun foo (Str $string = 'default') { ... }

# Specifying default hash value + Type check
fun foo(ArrayRef :$array = []) { ... }

For more examples of how to use Function::Parameters see here.

Types

Any class object can be used as a Type in Moose or with Function::Parameters, but the "primitive" types that are supported can be seen in the Moose Types Manual. These are:

Any
    Item
        Bool
        Maybe[`a]
        Undef
        Defined
            Value
                Str
                    Num
                        Int
                    ClassName
                    RoleName
            Ref
                ScalarRef[`a]
                ArrayRef[`a]
                HashRef[`a]
                CodeRef
                RegexpRef
                GlobRef
                FileHandle
                Object

Moose

Use Modern MooseX::* libraries

We should always use MooseX::Modern or MooseX::Modern::Role when defining classes and roles. These libraries import several useful attribute shortcuts that align with our best practices. They also automatically clean the namespace in which the Moose class is defined, so that only instance-based (OO) methods are exported.

package My::Role;
use MooseX::Modern::Role;

...
package My::Class;
use MooseX::Modern;
with 'My::Role';

...

Always make classes immutable

We should always be making classes immutable at the very bottom of our class definitions. Even if MOP (Meta-Object Protocol, or metaprogramming) is to be utilized, it is still preferable to explicitly mark a class as mutable, and then back to immutable once those MOP operations are complete.

# Class definition

__PACKAGE__->meta->make_immutable;

1;

Never override new

We should never override new. Instead, we use BUILD or BUILDARGS to hook into object construction.

The BUILDARGS method is called before an object has been created, and the primary purpose of BUILDARGS is to allow a class to accept something other than named arguments.

The BUILD method is called after the object is constructed, but before it is returned to the caller, and provides an opportunity to check the object state as a whole. This is a good place to put logic that cannot be expressed as a type constraint on a single attribute.

Here is an excellent example in the Moose cookbook for using both methods.

Enforce immutable state

We should enforce read-only attributes for immutable state. All attributes are defaulted to read-only thanks to an attribute shortcut that we use internally, so the ro keyword is not explicitly needed in new attribute definitions.

If an attribute must be read-write, it is preferable to define the writer as a separate private method with the rwp shortcut, as narrower APIs are much easier to maintain.

# Writer automatically installed as `_set_pizza`
has pizza => (
    is  => 'rwp',
    isa => 'Pizza',
);

Encourage laziness

We encourage lazy for calculated attributes unless those attributes are required or have trivial defaults. Setting is => lazy automatically install a builder for the attribute, and is the recommended approach over an anonymous default sub.

has size => (
    is      => 'lazy',
    isa     => 'Str',
    builder => method {
        return (qw/small medium large/)[ int(rand 3) ];
    },
);

DBIC

Tests

Unit test with t::lib::Unit

For adding additional unit test coverage to existing code, or for new greenfield deevelopment, make sure to use t::lib::Unit to take advantage of the most streamlined imports and mocking facilities, and minimize boilerplate in test code.

use t::lib::Unit;

Use MO and MM for mocking

The MO function is a handy shortcut to quickly creating Test::MockObject objects, and MM is a clean, scoped wrapper around Sub::Override, both of which can save a lot of time mocking common objects and interfaces.

# Mock Object
my $mock_user = MO(
    country_code      => 'US',
    email             => 'test@email.com',
    guid              => 'USR12345',
    registration_date => fun { DateTime->now->iso8601 },
);

# Mock Module
my $mock_request_id = MM('Crowdtilt::Internal::API',
    get_request_id => 'fake_request_id',
);

Postgres