The Clean Code Talks - "Global State and Singletons"

By: GoogleTechTalks

904   24   139065

Uploaded on 11/18/2008

Google Tech Talks
November 13, 2008

ABSTRACT

The Clean Code Talk Series

Speaker: Misko Hevery

Comments (8):

By anonymous    2017-09-20

Instead of that dreadful abomination, you should learn how to utilize spl_autoload_register():

spl_autoload_register( function( $classname ){

    $filename = 'inc/classes/' . $classname . '.class.php';

    if ( !file_exists( $filename) ){
        throw new Exception("Could not load class '$classname'.". 
                            "File '$filename' was not found !");
    }

    require $filename;

});

And you should register the autoloader in your index.php or bootstrap.php file, and do it only once per loader (this ability lets you define multiple loaders, but that's used, when you have third party library, which has own autoloader .. like in case of SwiftMailer).

P.S. please learn to use prepared statements with MySQLi or PDO.

Update

Since you are just now learning OOP, here are few things, which you might find useful:

Lectures:

Books:

Original Thread

By anonymous    2017-09-20

Since you request a way to mock a DataContext I assume that you really want to do some unit tests and not integration tests.

Well, I will tell you how to accomplish this, but first I would like to encourage you to read the following links, they are all about writing clean testable code.

And check the links from this response:

Watch the clean code talks from Misko Hevery (given to the Google people)

One thing that I used to repeat to myself and to my fellows at work, is that anyone can write a unit test, because they are silly easy to write. So a simple test is essentially all about making some comparisons and throw exceptions if the results fails, anyone can do that. Of course, there are hundreds of frameworks to help us write those tests in an elegant way. But the real deal, and the real effort shroud be put on learn how to write clean testable code

Even if you hire Misko Hevery to help you write tests, he will have a real hard time writing them if your code is not test-friendly.

Now the way to mock a DataContext objects is: do not do it

Instead wrap the calls using a custom interface instead:

public interface IMyDataContextCalls
{
    void Save();
    IEnumerable<Product> GetOrders();
}
// this will be your DataContext wrapper
// this wll act as your domain repository
public class MyDataContextCalls : IMyDataContextCalls
{
    public MyDataContextCalls(DataClasses1DataContext context)
    {
        this.Context = context;
    }

    public void Save()
    {
        this.Context.SubmitChanges();
    }

    public IEnumerable<Product> GetOrders()
    {
        // place here your query logic
        return this.Context.Products.AsEnumerable();
    }


    private DataClasses1DataContext Context { get; set; }

}

// this will be your domain object
// this object will call your repository wrapping the DataContext
public class MyCommand
{
    private IMyDataContextCalls myDataContext;
    public MyCommand(IMyDataContextCalls myDataContext)
    {
        this.myDataContext = myDataContext;
    }

    public bool myDomainRule = true;

    // assume this will be the SUT (Subject Under Test)
    public void Save()
    {
        // some business logic
        // this logic will be tested
        if (this.myDomainRule == true)
        {
            this.myDataContext.Save();
        }
        else
        {
            // handle your domain validation  errors
            throw new InvalidOperationException();
        }
    }
}

[TestClass]
public class MyTestClass
{
    [TestMethod]
    public void MyTestMethod()
    {
        // in this test your mission is to test the logic inside the 
        // MyCommand.Save method
        // create the mock, you could use a framework to auto mock it
        // or create one manually
        // manual example:
        var m = new MyCommand(new MyFakeDataContextFake());

        m.Invoking(x => x.Save())
            //add here more asserts, maybe asserting that the internal
            // state of your domain object was changed
            // your focus is to test the logic of the domain object
            .ShouldNotThrow();

        //auto mock example:
        var fix = new Fixture().Customize(new AutoMoqCustomization());
        var sut = fix.CreateAnonymous<MyCommand>();
        sut.myDomainRule = false;

        sut.Invoking(x => x.Save())
            .ShouldThrow<InvalidOperationException>();
    }

    public class MyFakeDataContextFake : IMyDataContextCalls
    {
        public void Save()
        {
            // do nothing, since you do not care in the logic of this method,
            // remember your goal is to test the domain object logic
        }

        public IEnumerable<Product> GetOrders()
        {
            // we do not care on this right now because we are testing only the save method

            throw new NotImplementedException();
        }
    }
}

Notes:

  • When you declare your IMyDataContextCalls interface you are actually abstracting the use of a DataContext, therefore this interface should contain only POCO objects (most of the time), if you follow this approach your interfaces will be decoupled from any undesired dependency.

  • In the specific MyDataContextCalls implementation, you are explicitly using a DataClasses1DataContext context, but you are free to change the implementation at any time and that won't affect your external code, and that's because you are always working with the IMyDataContextCalls interface instead. So at any time you could change for example this implementation for another one using the wonderful NHibernate =) or the poor ef or a mock one

  • At last, but not least. please double check my code, and you will notice that there are no new operators in the domain objects. This is a rule of dumb when writing test friendly code: decouple the responsibility of creating objects outside of your domain objects


I personally use three frameworks on every project and on every test I write, I really recommend them:

For example, in the code above, I showed you how to write a manual fake for your repository, but that clearly is something we do not want to do in a real project, imagine the number of objects you would have to code in order to write your tests.

Instead use the power of AutoFixture combined with Moq:

This line: var m = new MyCommand(new MyFakeDataContextFake());

Will become:

        var fixture = new Fixture().Customize(new AutoMoqCustomization());
        var sut = fixture.CreateAnonymous<MyCommand>();

And that's it, this code will automatically create mocks for all the objects needed in the constructor of MyCommand.

Original Thread

By anonymous    2017-09-20

I know the "mydata" pointer will be just available on this particular cpp file

That's false. mydata will be a global variable. If you have another mydata global variable in another .cpp, they can collide (or at least they will violate the one definition rule).

Use unnamed namespaces for mydata:

namespace {
    myData* mydata = nullptr;
}

This way, mydata won't be accessible to other translation units.

But: global variables are not recommended. Don't use them. They almost always have alternatives.

There is a technical reason, which related to C++ only: static initialization order fiasco

And there are design decisions behind:

Original Thread

By anonymous    2017-11-06

I am learning the popular MVC and trying to implement it in PHP. I am designing a framework with a pure OOP fashion (though I am not expert in PHP's OOP ability. I only have moderate knowledge about it). An example implementation of this framework as shown in the following figure. enter image description here

In this framework I added a Data Access Layer, DAL, (a class to deal with the connection, executing query and transport to and from the database) to abstract the physical database from the rest of the system to easy change of the data source. If a system is only bind to one database of one particular type, this layer is expected to presented in the system using only one object with one connection to the database. And this object will be a dependency for all the Data Mapper objects (i.e., User mapper, Product Mapper).

I am looking for your comments on where to initiate the DAL object in the system. I can create the object in the front controller (index.php) and transport all the way to Data Mapper objects. But it is an anti-pattern according to Here and Here. Even for the same reason, we cannot initiate the DAL object within the factories (Factories can be separated in multiple classes for handling complexities as per Clean code approach ). I cannot use Singleton as that is also going to create lots of problem according to this. So, in your opinion, what is the best practice and place where I can initiate this object and pass it to Data Mapper objects?

N.B.: I disregard the View Logic here as my concern do not have any relation with Views.

Original Thread

By anonymous    2018-02-18

Static, singleton, global

I am sorry to disappoint you, because I am sure you invested a lot of effort in your code,... but you could or should take in consideration to start by not using any static methods (nor singletons or global variables). Here and here are some reasons presented.

Dependency injection (DI)

Inside a class, don't use methods of another class to create instances. If you need an object of any type, inject it - in constructor, or in any other method that needs it. This is called dependency injection. See this and this.

So, you should have this (further below I will change it):

class LoginController {

    private $template;

    __construct(Template $template) {
        $this->template = $template;
    }

    public function getView() {
        $this->template->renderTemplate('index.html');
    }

}

Dependency injection container (DIC)

The structure that has the responsibility of injecting dependencies (e.g. wiring) is called dependency injection container (DIC). Examples: PHP-DI, or Auryn). It is constructed and used only at the entry point of your app (like in index.php or bootstrap.php). In it you will register your app dependencies, and it will automatically take care of injecting them all over. Your App class would then be redundant. And your application will "concentrate" itself on its actual purpose, not on the creation and/or implementation of the structures that it requires in order to be able to complete its tasks. The related term is Inversion of Control (IoC).

Service locator

As a note: don't pass the DIC as dependency. If you do that you are "creating" a so called service locator. It would mean that your class would depend on the container to fetch ("locate") some of its dependencies, or to run some of its processes through it.

Tight coupling

In analogy to the previous point: don't create objects using the new keyword inside a class, because you are making the class dependent of another - tight coupling. See this. The solution is the same as above: the dependency injection.

Btw, in your code you are coupling each controller to the App class as well. As mentioned, this should be avoided.

So, in the Template class you should simply have:

class Template {

    private $environment;

    public function __construct(Twig_Environment $environment) {
        $this->environment = $environment;
    }

    public function render($template, array $vars) {
        return $this->environment->render($template, $vars);
    }

}

and let the creation of the Twig environment to be performed in some other place in your code (e.g. by the DIC). Note the return statement, instead of echo: let the output of the rendered content happen on a higher level, e.g. at the bootstrap level (like index.php, or bootstrap.php) - see last code below.

Library for the main classes

Having said that, you could have a folder (like library or framework) on the project root niveau, in which all core classses such as the following could reside: Template, View, model layer constructs (like data mappers, repositories, db adapters, services, etc), Config, Session, HTTP related classes (Request, Response, Stream, Uri, etc). See this and this.

Routing

I see a routes.php file in your structure. So I am assuming that you are already familiar with the fact that the "translation" (parsing) of an URI - passed into the client (a browser) - into a route object containing the informations about a specific controller action (including the action parameters) is the responsibility of a router (most likely composed of more classes: Route, Dispatcher, Router, etc). Example of implementations: FastRoute, Aura.Router.

Front controller (request dispatching)

The task of dispaching the request, e.g. of finding - and, eventually, creating - a route object in a predefined routes collection (based on the provided URI) and calling the controller action (with the action parameters as arguments), is of the so called front controller. It will receive the router and a dispatcher as dependencies. In short, this front controller grabs a route object from the router and passes its informations to the dispatcher, which is responsible for calling the action. An example:

class FrontController {

    private $router;
    private $dispatcher;

    public function __construct(Router $router, Dispatcher $dispatcher) {
        $this->router = $router;
        $this->dispatcher = $dispatcher;
    }

    public function routeAndDispatch(ServerRequestInterface $request, ResponseInterface $response) {
        $route = $this->router->route($request->getMethod(), $request->getUri()->getPath());
        $this->dispatcher->dispatch($route, $request, $response);
        return $this;
    }

}

A good tutorial on this theme is presented in here, and the continuation here.

HTTP message abstraction (PSR-7)

Having in mind the nature of the web applications, based on request and response, you should familiarize yourself a bit with the PSR-7 recommendation. It provides an abstraction of the HTTP message - composed of a HTTP request and a HTTP response. A good standalone library is Zend-Diactoros.

Namespaces vs. file system structure (PSR-4)

Regarding the file system/namespaces structure: As per PSR-4 recommendation, a file with a class in it should have the same name as the class. Also, if you use imports (use keyword), then you don't have to use the fully qualified class name in other places of the code too. So, correct is like:

namespace App\Controllers\Frontend\Guest;

use App\App;

App::getProvider(...)->...;

Controllers and views - 1:1 relation

Please note that a view-controller relation could be 1:1. Their action methods would be then called separately (in the front controller):

// Call the controller action.
call_user_func_array([$controller, "login"], <action-params>);

// Call the view action.    
call_user_func_array([$view, "login"], <action-params>);

// Call the output method of the view and print.
echo call_user_func_array([$view, "output"]);

In this constellation both, controller and view, can share different objects - like services, or domain objects (by many called "models"), or data mappers, etc. Let's say, that a controller and a view share a domain object - for example a "User" object. That means that both receive a certain domain object as constructor argument. This object will be, of course, automatically injected by the DIC.

First separation of concerns

By doing so, a first separation of concerns takes place:

  • The controller changes the state of the domain object (e.g. the value of its properties) and no more than that. No more "presentation"-related tasks...
  • All these "presentation"-related tasks remain the duty of the view - as it should, isn't it? And because the view receives the same "User" object as argument, it has access to the state values already set/changed by the controller. In the situation here the domain object is responsible with the db interaction too - not good though. Anyway, the view puts the template instance to work, requests the domain object to query the db and to return the results, and, in the end, returns the rendered content to be presented on screen with a simple echo.

Like:

class PdoAdapter implements AdapterInterface {

    private $connection;

    public function __construct(PDO $connection) {
        $this->connection = $connection;
    }

    public function fetchColumn(string $sql, array $bindings = [], int $columnNumber = 0) {
        $statement = $this->connection->prepare($sql);
        $statement->execute($bindings);
        return $statement->fetchColumn($columnNumber);
    }

}

class LoginController {

    private $user;

    public function __construct(UserInterface $user) {
        $this->user = $user;
    }

    public function login($name, $password) {
        $this->user->setName($name);
        $this->user->setPassword($password);
    }

}

class User implements UserInterface {

    private $adapter;
    private $name;
    private $password;

    public function __construct(AdapterInterface $adapter) {
        $this->adapter = $adapter;
    }

    public function getName() {
        return $this->name;
    }

    public function setName($name) {
        $this->name = $name;
    }

    public function getPassword() {
        return $this->password;
    }

    public function setPassword($password) {
        $this->password = $password;
    }

    public function checkLoggedIn($name, $password) {
        $sql = 'SELECT COUNT(*) FROM users WHERE name=:name AND password=:password LIMIT 1';
        $bindings = [
            ':name' => $name,
            ':password' => $password,
        ];

        return $this->adapter->fetchColumn($sql, $bindings) > 0;
    }

}

class LoginView {

    private $user;
    private $template;

    public function __construct(UserInterface $user, Template $template) {
        $this->user = $user;
        $this->template = $template;
    }

    public function login() {
        //...
    }

    public function output() {
        return $this->template->render('index.html', [
            'loggedIn' => $this->user->checkLoggedIn(
                $this->user->getName()
                , $this->user->getPassword()
            ),
        ]);
    }

}

Second separation of concerns - the MVC goal

In order to further separate the tasks, use data mappers, as intermediaries between the domain objects and the db (or the persistence layers, in general). E.g. in order to transfer data between the domain objects and the db.

In this step, the business logic, represented in and by the domain objects, become completely separated from any other app components. With this step, the actual goal of the MVC pattern is achieved: the decoupling of business logic from any other app structures/processes.

This can be seen in the following example: the User entity contains now only its properties and the methods related to them. The db functionality belongs now to the data mapper.

<?php

class PdoAdapter implements AdapterInterface {
    // ... The same ...
}

class UserMapper implements UserMapperInterface {

    private $adapter;

    public function __construct(AdapterInterface $adapter) {
        $this->adapter = $adapter;
    }

    public function checkLoggedIn($name, $password) {
        $sql = 'SELECT COUNT(*) FROM users WHERE name=:name AND password=:password LIMIT 1';
        $bindings = [
            ':name' => $name,
            ':password' => $password,
        ];

        return $this->adapter->fetchColumn($sql, $bindings) > 0;
    }

}

class LoginController {

    private $user;
    private $mapper;

    public function __construct(UserInterface $user, UserMapperInterface $mapper) {
        $this->user = $user;
        $this->mapper = $mapper;
    }

    public function login($name, $password) {
        $this->user->setName($name);
        $this->user->setPassword($password);
    }

}

class User implements UserInterface {

    private $name;
    private $password;

    public function getName() {
        return $this->name;
    }

    public function setName($name) {
        $this->name = $name;
    }

    public function getPassword() {
        return $this->password;
    }

    public function setPassword($password) {
        $this->password = $password;
    }

}

class LoginView {

    private $user;
    private $mapper;
    private $template;

    public function __construct(UserInterface $user, UserMapperInterface $mapper, Template $template) {
        $this->user = $user;
        $this->mapper = $mapper;
        $this->template = $template;
    }

    public function login() {
        //...
    }

    public function output() {
        $loggedIn = $this->mapper->checkLoggedIn(
                $this->user->getName()
                , $this->user->getPassword()
        );

        return $this->template->render('index.html', [
                    'loggedIn' => $loggedIn
        ]);
    }

}

Further separations/optimizations

  • (Optional) Use repositories to intermediate between the domain objects and the data mappers.
  • Use services to interact with the domain objects and the data mappers/repositories.
  • Share the services between the controllers and the views.

This great answer describes this part in detail.

And as last thing: all these dependencies in the examples are managed by the dependency injection container.


How to wire all together (steps):

• Define all "config" definitions, which will be registered into the dic. I speak about configuration values only, which will be further read by other "general" definitions of the dic, in order to create objects. The "config" definitions will probably reside in configuration files (e.g. definition files). So, for example, the definition file for the twig components could look like this:

config/configs/twig.php:

<?php

return [
    'twig' => [
        'options' => [
            'debug' => TRUE,
            'charset' => 'UTF-8',
            'base_template_class' => 'Twig_Template',
            'strict_variables' => FALSE,
            'autoescape' => 'html',
            'cache' => __DIR__ . '/../../storage/cache/twig',
            'auto_reload' => TRUE,
            'optimizations' => -1,
        ],
        'extensions' => [/* Indexed array with Twig_Extension_xxx instances */
            new Twig_Extension_Debug(),
        ],
    ],
];

• Define all "general" definitions, which will be registered into the dic and are used to create objects. But I don't speak here about the "specific" definitions, used to create specific objects like controllers, views, domain ojects, data mappers, etc. The "general" definitions will probably reside in one file. For example:

config/generals.php:

<?php

// Here come the imports...

return [
    'router' => function (ContainerInterface $c) {
        $dispatcher = 'Here create the route dispatcher...';
        return new Router($dispatcher);
    },
    'request' => factory([ServerRequestFactory::class, 'build']), /* Built with defaults */
    'response' => function (ContainerInterface $c) {
        return new Response(new Stream() /* defaults */);
    },
    'twigEnvironment' => function (ContainerInterface $c) {
        $loader = new Twig_Loader_Filesystem();
        $options = $c->get('twig')['options'];
        $environment = new Twig_Environment($loader, $options);

        $extensions = $c->get('twig')['extensions'];
        foreach ($extensions as $extension) {
            $environment->addExtension($extension);
        }

        return $environment;
    },
];

• Define all "specific" definitions, which will be registered into the dic and are used to create objects. I speak here about the definitions used to create controllers, views, domain ojects, data mappers, etc, but only for the dependencies type-hinted with interfaces or not type-hinted at all ("skalar" parameters). These "specific" definitions will probably reside in one file:

config/specifics.php:

<?php

use function DI\get;
use function DI\object;
use Mymvc\Models\User\User;
use Mymvc\Views\Login\LoginView;
use Mymvc\Models\User\UserMapper;
use Mymvc\Controllers\Login\LoginController;

return [
    LoginController::class => object()
            ->constructorParameter('user', get(User::class))
            ->constructorParameter('mapper', get(UserMapper::class)),
    LoginView::class => object()
            ->constructorParameter('user', get(User::class))
            ->constructorParameter('mapper', get(UserMapper::class)),
];

These "specific" definitions are based on the following LoginController:

<?php

namespace Mymvc\Controllers\Login;

use Mymvc\Models\User\UserInterface;
use Mymvc\Models\User\UserMapperInterface;

class LoginController {
    public function __construct(UserInterface $user, UserMapperInterface $mapper) {
        //...
    }
}

and on the following LoginView:

<?php

namespace Mymvc\Views\Login;

use Mylib\Presentation\Template;
use Mymvc\Models\User\UserInterface;
use Mymvc\Models\User\UserMapperInterface;

class LoginView {

    public function __construct(UserInterface $user, UserMapperInterface $mapper, Template $template) {
        //...
    }

}

Important: The Template dependency of the LoginView must not be defined in the "config/specifics.php" file, because for all concrete type-hints the DIC automatically creates instances! In this so-called autowiring process lies the real power of any DIC.

Note that get & object in "config/specifics.php" are functions of the DIC.

• Define all routes, which will be registered into the router. They could reside in config/routes.php

• In index.php only include the bootstrap file. So, index.php contains only one line:

<?php require_once '../bootstrap.php'; ?>

From here on the work is made in bootstrap.php, which resides directly in the project root.

• Load Composer autoloader.

• Create an instance of the DIC builder.

• Register all previously defined definitions into the DIC.

• Compile the container, e.g. the DIC.

• Get the router from the DIC.

• Register all previously defined routes into the router.

• Read the uri from the browser.

• Pass it to the router and call the dispatch method of the router. The router compares the uri with each registered route and, if it finds a match, it will return an array with the informations contained in the registered route ("route infos" array).

• The route infos are: the controller name, the action name and the action parameters list.

• Get the request object from the DIC by reading the 'request' entry from it and get the response object from the DIC by reading the 'response' entry from it. The generated response object will be probably used along the way before the controller action is called. For example, if you want to create a controller instance, but no controller class exists, then you'll use the response object to output an error message or a customized error page.

• Save the route infos into the request object as "attributes", e.g. with the withAttribute() method of the PSR-7 recommendation. From hear on read them only from the request object.

• Register the request object into the DIC by setting an entry ServerRequestInterface::class into it with the value of the request object. This way, because of autowiring, each time a dependency ServerRequestInterface is needed everywhere the assigned request object containing the route infos will be automatically injected.

$container->set(ServerRequestInterface::class, $request);

• If you don't use the response object for some operations before calling controller/view action anymore, then you can already register it into the DIC by setting an entry ResponseInterface::class. This way, because of autowiring, each time a dependency ResponseInterface is needed everywhere the assigned response will be automatically injected.

$container->set(ResponseInterface::class, $response);

• If an object of a certain type don't need to perform operations before the controller/view action is called, then you can register it as entry of DIC from the beginning. Otherwise, similar with request/response objects, create an instance of that type, use it, then register it afterwords. For example, a session object is needed to perform some session functions. After these ops are finished, a 'Session::class` entry can be registered in DIC.

• Based on the route infos and on the paths/namespaces registered as configs (e.g. "config" definitions in DIC) build the fully qualified class name (FQN) of the controller and of the view.

• Call the controller action. If not, no problem: the view action of the view is called. Make also proper validations (file/class exist, method exists). But use the call() method of the container (e.g. DIC), so that the automatic injection takes place!

• Here is the most awaited moment: create a Template object (by using the configs/paths/namespaces registered in DIC inclusive the 'twigEnvironment' entry) and register it into DIC. For example:

In config/configs/app.php:

<?php

return [
    'paths' => [
        // This folder contains the templates and the layouts of the application, used by your view classes if no "specific" layouts/templates are used.
        'appTemplates' => __DIR__ . '/../../resources/templates/app'
        // This folder contains the templates and the layouts loaded/rendered by the specific view classes (LoginView, HomeView, AboutUsVIew, etc).
        'customTemplates' => __DIR__ . '/../../resources/templates/app'
    ],
];

And now in bootsptrap.php:

$appTemplatesPath = $container->get('appTemplates');
$customTemplatesPath = $container->get('customTemplates');
$twigEnvironment = $container->get('twigEnvironment');

$template = new Template(
    $twigEnvironment
    , $appTemplatesPath
    , $customTemplatesPath
);

$container->set(Template::class, $template);

• Call the view action if exists. If not, no problem: the 'output' method of the view is called. Make also proper validations (file/class exist, method exists). And use the call() method of the container (e.g. DIC), so that the automatic injection takes place!

• Call the 'output' method of the view and print the result with echo. If none exists then throw an exception: the program is broken. Make also all proper validations (file/class exist, method exists). And use the call() method of the container (e.g. DIC), so that the automatic injection takes place!

• The end...

Good luck.

Original Thread

Popular Videos 335

Submit Your Video

If you have some great dev videos to share, please fill out this form.