Posts

This article is part of the new series “How to unclutter your controllers”. This post shows you how to cleanup your code by using policy objects. I use the PHP with a Laravel flavour but you can easily adapt the patterns to another language and framework that follows the glorious MVC pattern. Don’t care to much about the used methods like response() if you’re not familiar with it.

The idea

Policies can be used to decouple your business rule implementation (rules for data validity, protection of actions) from the actual controller logic (Single Responsibility Principle).

How it works

Let’s whip up a small example scenario where you are a company that creates amazing and beautiful fluffy cats. A creation method could look like this:

PHP

1

2

3

4

5

6

7

8

9

10

classCatsController{

publicfunctionstore(Request$request)

{

$fluffyCat=FluffyCat::create($request->all());

returnrespond($fluffyCat);

}

}

Sweet, but now your boss decided to allow only 5 black cats to keep the world in balance? I’m pretty sure you will end up with something like this:

PHP

1

2

3

4

5

6

7

8

9

10

11

12

13

14

15

16

17

18

19

classCatsController{

constMAX_NUMBER_BLACK_CATS=5;

publicfunctionstore(Request$request)

{

if($request->input('color')=='black'){

$numberOfBlackCats=Cat::where('color','black')->get()->count();

if($numberOfBlackCats>=self::MAX_NUMBER_BLACK_CATS){

returnresponse(403,'nah_nah_nah_no_black_cat_for_you');

}

}

$fluffyCat=FluffyCat::create($request->all());

returnrespond($fluffyCat);

}

}

That code looks gross doesn’t it?

I mean okay, it still looks kind of okay, but what happens if your boss wants to reduce the number of cats in the world even more by some other silly rules?
Or if you just don’t want to allow the particular user to have cats? (I mean everything is possible right?). You see!

Abstraction to the rescue!

Let’s try to refactor that tiny little function that got a little bit out of wag:

PHP

1

2

3

4

5

6

7

8

9

10

11

12

13

14

15

16

17

18

19

20

21

22

23

24

25

26

27

28

29

30

31

32

33

34

35

36

37

38

39

40

41

42

43

44

45

46

47

48

49

50

51

52

53

54

55

56

57

58

interfaceHasPolicies{

protected$policy;

publicfunctionhasError($message,$code);

}

classCatsControllerimplementsHasPolicies{

protected$policy;

publicfunction__construct()

{

$this->policy=newCatsPolicy($this);

}

publicfunctionstore(Request$request)

{

$this->policy->canStoreColor($request->input('color'));

$fluffyCat=FluffyCat::create($request->all());

returnrespond($fluffyCat);

}

publicfunctionhasError($message,$error)

{

returnrespond($error,$message);

}

}

classPolicy{

protected$instance;

publicfunction__construct($instance){

$this->instance=$instance;

}

publicfunctionhasError($message,$code=403)

{

$this->instance->hasError($message,$code);

}

}

classCatsPolicyextendsPolicy{

constMAX_NUMBER_OF_CATS=5;

publicfunctioncanStoreColor($color)

{

$numberOfFluffyCats=FluffyCat::where('color',$color)->get()->count();

if($numberOfFluffyCats>self::MAX_NUMBER_OF_CATS){

$this->hasError('no_black_cats_allowed_anymore',403);

}

}

}

uff okay, that looks quite complex with 2 new classes and interface in the mix. Scary… But let’s go through this together:

So we learned that a Object should have only one responsibility.
That means that our controller should do only one thing:

Controlling things: Telling the rest of the application “Hey, there is a (http) request, please do something with it!”.
So the first thing we do is to give the rules a new home by extracting it a dedicated policy class:

PHP

1

2

3

4

5

6

7

8

9

10

11

12

13

classCatsPolicyextendsPolicy{

constMAX_NUMBER_OF_CATS=5;

publicfunctioncanStoreColor($color)

{

$numberOfFluffyCats=FluffyCat::where('color',$color)->get()->count();

if($numberOfFluffyCats>self::MAX_NUMBER_OF_CATS){

abort('you_not_alowed_to_create_a_black_cat',403);

}

}

}

The controller get’s it’s brand new policy class injected:

PHP

1

2

3

4

5

6

7

8

9

10

11

12

13

14

15

16

17

18

classCatsController{

protected$policy;

publicfunction__construct()

{

$this->policy=newCatsPolicy;

}

publicfunctionstore(Request$request)

{

$this->policy->canStoreColor($request->input('color'))

$fluffyCat=FluffyCat::create($request->all());

returnrespond($fluffyCat);

}

}

That already looks way better but there is still some room for improvement: The Policy requires a http compatible companion to fullfill it’s duty. (the policy consumer must use a http type of error handling).

We have different possibilities to tackle this like throwing errors and propagating the error to the controller. But why don’t we try something else today! Nothing can stop us to just pass the controller instance to the policy right?

PHP

1

2

3

4

5

6

7

8

9

10

11

12

13

14

15

16

17

18

19

20

21

22

23

24

25

26

classCatsPolicyextendsPolicy{

constMAX_NUMBER_OF_CATS=5;

protected$instance;

publicfunction__construct($instance)

{

$this->instance=$instance;

}

publicfunctioncanStoreColor($color)

{

$numberOfFluffyCats=FluffyCat::where('color',$color)->get()->count();

if($numberOfFluffyCats>self::MAX_NUMBER_OF_CATS){

$this->handleError('you_not_alowed_to_create_a_black_cat',403);

}

}

privatefunctionhandleError($message,$code)

{

return$this->instance->hasError($message,$code);

}

}

The beauty of this: We can now put the “HTTP” logic inside the controller where it belongs:

PHP

1

2

3

4

5

6

7

8

9

10

11

12

13

14

15

16

17

18

19

20

21

22

23

24

classCatsController{

protected$policy;

publicfunction__construct()

{

$this->policy=newCatsPolicy($this);

}

publicfunctionstore(Request$request)

{

$this->policy->canStoreColor($request->input('color'))

$fluffyCat=FluffyCat::create($request->all());

returnrespond($fluffyCat);

}

publicfunctionhandleError($message,$err)

{

returnabort($message,$err);

}

}

You could already stop there if you like. but If you want to improve the code even more you can consider several extra steps:

Give the Policy constructor a uniform home in a base class

Create an interface for the Controller

Using a trait (example in PHP)

Provide the policy an array of rules instead of using single function calls.

Give the Policy constructor a uniform home in a base class

Let’s create the base class:

PHP

1

2

3

4

5

6

7

8

9

10

11

12

13

14

classPolicy{

protected$instance;

publicfunction__construct($instance)

{

$this->instance=$instance;

}

privatefunctionhandleError($message,$code)

{

$this->instance->hasError($message,$code);

}

}

And inherit from it like this:

PHP

1

2

3

4

5

6

7

8

9

10

11

classCatsPolicyextendsPolicy{

publicfunctioncanStoreColor($color)

{

$numberOfFluffyCats=FluffyCat::where('color',$color)->get()->count();

if($numberOfFluffyCats>self::MAX_NUMBER_OF_CATS){

$this->handleError('you_not_alowed_to_create_a_black_cat',403);

}

}

}

You see, now we have a super tiny policy class.

Create interfaces

The interface tells the controller which methods and attributes need to be implemented. Now you can make always sure that the hasError method exists:

PHP

1

2

3

4

5

6

7

8

9

10

11

interfaceHasPolicy{

publicfunctionhasError($message,$err);

}

classCatsControllerimplementsHasPolicy{

// the same code as aboth, but the interpreter will tell you that the method hasError must exist and the attribute policy available

}

Using a trait (PHP only)

PHP also supports traits. These are nice and small includable code snippets for your class. It helps you to use Composition over Inheritance quite easy. Let’s use it for populating our policy logic to the target controller. Inheritance over a base class would be also a feasible way:

PHP

1

2

3

4

5

6

7

traitUsesPolicies{

protected$policy;

publicfunctionhasError($message,$code)

{

abort($message,$code);

}

}

Now you can get rid of your hasError method inside the controller when you use the “normal way of error handling”. It also takes care of the contract between the policy and the controller so you don’t even need the interface anymore:

PHP

1

2

3

4

5

6

7

8

9

10

11

12

13

14

15

16

17

18

19

classCatsController{

useUsesPolicies;

publicfunction__construct()

{

$this->policy=newCatsPolicy($this);

}

publicfunctionstore(Request$request)

{

$this->policy->canStoreColor($request->input('color'))

$fluffyCat=FluffyCat::create($request->all());

returnrespond($fluffyCat);

}

}

Apply method

When the rules are getting more and more consider a apply method that handles an array of rules and calls the corresponding methods inside your Policy class:

PHP

1

2

3

4

5

6

7

8

9

10

11

12

13

14

15

16

17

18

19

20

21

22

23

24

25

26

27

28

29

30

31

32

33

34

traitUsesPolicies{

protected$policy;

publicfunctionhasError($message,$code)

{

abort($message,$code);

}

protectedfunctionapply($rules=[],$request)

{

foreach($rulesas$rule){

$this->policy->{$rule}($request);

}

}

}

classCatsController{

useUsesPolicies;

publicfunctionstore(Request$request)

{

$this->policy->apply([

'canStoreColor',

'dontAllowDogOwnersToCreatACat',

'andAnotherImportantRule'

],$request);

$fluffyCat=FluffyCat::create($request->all());

returnrespond($fluffyCat);

}

}

Conclusion

Patterns are amazing! It’s a bit of extra work at the beginning but if you use this technique in all of your controllers it simply adds up.

I hope that the above examples are helped you to increate the quality of your code.

Introduction

As a freshly onboarded intern of the Zenguard team the first weeks were, as with most introduction periods, mainly about getting up to speed, getting familiar with the workflow, setting up your development environment and gaining access to testing and production environments. After this period it is time to develop features, fix bugs and work on tasks, issues and stories. Before any of these features, get deployed to our QA team and after QA to the actual production release, the code gets reviewed… of course.

For someone like me, who just finished his application development studies, and having little to no experience with reviewing code, code reviews can be intimidating and I found it personally hard to start participating in them. Questions like “What do I look for when reviewing code?” and “Would this be actual valid input?” come to mind.

After asking some questions about how we do code reviews eg: “are there any guidelines that I can follow?”, we came to the conclusion that their is room for improvement in our current way of reviewing code. This blog post is about code reviews in general, how we used to do code reviews and how we do our code reviews now.

Code reviews in general

As mentioned before code reviews can be intimidating, especially when you are new to code reviewing in general. The most important point in my opinion is to lose this attitude, and realize what code reviews are really about:

Improving and maintaining a clean and structured code base.

Learning what and why changes are made in the code base.

Giving new programmers the opportunity to learn and ask questions.

Sharing knowledge.

Based on these points any comments, change requests and criticism on your or somebody else’s code should be appreciated, and will ultimately result in a better code base, growth in expertise and will help to create and maintain a positive code review culture in which finding defects is considered a good thing.

How we used to do it

After my onboarding period and having worked on some tickets, I found that it was time to start participating in reviewing our code. We had some general rules, for example: “Pull requests should be reviewed by at least 2 developers before merging with the base branch” but no concrete processes or scheduled code review sessions.

We do our code reviews on Github and our general flow is as follows:

The developer finds an issue to work on, then creates a new branch and names it appropriately

After finishing the issues, this can be a feature, bug fix etc. The developer creates a pull request and the integration tests and code quality tests are ran.

At this point the pull request is open to be reviewed by our other developers, if the code gets approved by 2 developers the pull request can be merged.

If there are any requests for changes, they will be commented on the concerned file and lines. The developer sees these requests, changes the code appropriately, pushes the changes and the code review process starts again.

How we do it now

Because we are working in a small development team the process that that was already in place worked fine, and instead of changing the process I like to believe that we’ve upgraded it. After doing research on how other companies do code reviews, and there are a lot of resources on this topic online, we’ve scheduled a meeting and came up with the following process to review our new developed code:

Code should be reviewed by at leas 2 developers before merging.

Use a checklist and established guidelines while reviewing code, our checklist can be found below.

Weekly developer jour fix where we discuss technical debts and do code reviews: developers show examples of code what in their opinion can be improved and we discuss the best way to do so.

We are currently still doing or code reviews on Github and so far we have not found it necessary to purchase or use another external service specialized for code reviews. Github makes it easy to to add integration and code quality tests to pull requests and the code review interface makes it easy to comment on, or request changes to code.

Conclusion

Although we already were reviewing code and had some guidelines for doing code reviews, it is always nice to be able to improve these processes and streamlining it which hopefully increases productivity in the long run by keeping a clean and organized code base were developers know what, when and why changes were made it the existing code base.

This is by no means the best way to review code and please feel free to suggest ideas and or improvements, I hope this blog post will be interesting for developers who are new to code reviews or small development teams who are struggling with doing code reviews or establishing processes for it.

Code reviews are important! You should define goals and have standards, but keep in mind what the overall purpose is. Don’t be picky, don’t review code for hours straight, stay polite and as a team try to build a comfortable atmosphere which will result in making your own working life easier.