2020 02 09 Responsibilities

Difficult finding responsibilities

What is responsible for what?

This project is like to experiment with new ideas to lead in a simple maintainable code.
This time I fighting with the responsibilities. My scenario is a click on the menu button. Yes, that sounds simple but here many developer do dangerous stuff in. (So far my daily experience)

The wanted actions are:

• Stop event bubbling
• Load new module

And here are the first mistake. Stop event bubbling is not an action. Correct requirement is:

• Avoid default action at menu click
• Load module

Now we come to the responsibility. Classical many developers does like:

class SideMenu {
...
  render() {
      return <a onClick={(e)=>{e.preventDefault(); e.stopPropagation(); ModuleLoader.load(Registry.getPage('name').module);}} >...</a>;
  }
...
}

But that is in my eye not clean and mostly difficult ro read and can only find after longer debugging session . Next version would the be like:

class SideMenu {
...
  onMenuClick(event, menu) {
    e.preventDefault();
    e.stopPropagation (); 

    const module = Registry.getPage(menu).module;
    ModuleLoader.load(module)
  }

  render() {
      return <a onClick={(event)=>{this.onMenuClick(event, 'name')}} >...</a>;
  }
...
}

For better understanding, lets switch to TypeScript:

class SideMenu {
...
  onMenuClick(event: React.MouseEvent<HTMLAnchorElement>, name: string) {
    event.stopPropagation();
    event.preventDefault();
    const module = Registry.getPage(menu).module;
    ModuleLoader.load(module)
  }

  render() {
      return <a
        onClick={(event: React.MouseEvent<HTMLAnchorElement>) => this.onMenuClick(event, name)} 
      >
        ...
      </a>;
  }
...
}

That seems more clean now. Someone will maybe say: "That's more code to write for nothing!". That remember, we will write code, which is maintainable. And the code is the documentation(Uncle Bob). 😉

But if we test it, it become a bigger code:

...
  let menuOpenState: IObserver<boolean>, router: Router,
    routerRegistry : RouterRegistry, menus:any;

  beforeEach(() => {
    router = jest.genMockFromModule<Router>('../Router/Router');
    routerRegistry = jest.genMockFromModule<RouterRegistry>('../Router/Registry');
    menus = [{
      name: 'home',
      url: './home/',
      depth: 1
    }, {
      name: 'settings',
      url: './settings/',
      depth: 1
    }];
  });

  it('Manage menu clicks', async () => {
    const instance: RenderResult = render(<SideMenu model={model} router={router} routerRegistry={routerRegistry}/>);

    const event: React.MouseEvent<HTMLAnchorElement> | MouseEvent = new MouseEvent('click', {
      bubbles: true,
      cancelable: true
    });
    event.stopPropagation = jest.fn();
    const page: IPageData = {
      depth: 0,
      name: 'settings',
      url: 'url'
    };
    routerRegistry.getPages = jest.fn().mockReturnValue({name: page});
    router.changePage = jest.fn();

    const settingsMenuElement: HTMLElement = await instance.findByTestId('settings');
    fireEvent(settingsMenuElement, event);

    expect(event.stopPropagation).toHaveBeenCalled();
    expect(event.defaultPrevented).toBeTruthy(); // preventDefault() not mock able
    expect(adapter.onMenu).toHaveBeenCalledWith('settings');
    expect(router.changePage).toHaveBeenCalledWith(page);
  });
...

>> To shorten it, I just draft this code ...

What we see it, we must prepare:

• Menu data
• Router mocks
• Event mocks
• View

But let's come back, to what we want!

We just want test a menu click and change the page.
Here we find our next mistake! Do we want click and change page? Yes, from the point of stakeholder we want. But technical is it a difference! And so we have to find definition for responsibilities:

• SideMenu - View: Show menu and interact with browser
• Router: Modify browsers history
• Page Observer: State which contains current page data (comparable with redux state)
• RouterRegistry: Memory of existent pages.
• ModuleLoader: Load a js-Module by page data.

Wow, that a lot different stuff involved in our case. So how we can split up? Lets divide the code in his stereotypes:

• Visual Rules
  • SideMenu
• Business Rules
  • ???
• Processing Rules
  • Router
  • Page Observer
  • RouterRegistry
  • ModuleLoader

Stop! Where are the business rules? And here we found our problem that we do, if we implement stakeholders requirements directly. We ignored the business rule implementation.

But what should here? In fact, define our stakeholder just business rules. Maybe also the visual rules(how is should look and behave). How it proceed technical the stakeholder will,can and should not know. That developer business 😉

Ok...back to rules. What is now the missing business rule?
>> When menu is clicked, then change the url and load the page.

And!... we have the next mix-up ... so correct rules to:
>> 1) When menu is clicked, than change page.
>> 2) Is page changed, that change url.
>> 3) Is page changed, than load module.

Point 1) and 3) was defined in current ticket(Srcum, Kanban, etc). Point 2) is implement by someone else. If we take now look in our test, we realize, that we testing point 2) also.

Now we got the metric, what we need to split. I want to implement only 1) and 3). So I should only have that one in my unit.

How we can take 2) out of my code, without loosing the function?

We need a weak dependency between the code for the rules. That can be done in many ways. Mostly the developers took all dependencies into the React-Context and transport them to any place. My solution goes a bit more detailed. I use the idea of dependency injection and build some singleton container. In that I implement the dependencies out of any other code. So we got weak dependencies between our units.
As next we have to find a place for the business rules. The code of them is typically a controller pattern. That can be interactors for domain based business rules or just actions on application level.

I stay in begin of the project(2020er Version) and decide for actions:

• Visual Rules
  • SideMenu
• Business Rules
  • Application Action
• Processing Rules
  • Router
  • Page Observer
  • RouterRegistry
  • ModuleLoader

To make this text more understandable, we ignore from now on the processing rules. (Its even not part of out task 😉)

Only from now on, we can realize, that we have to implement: The handling for the HTML "encoding" and UI "input" in the menu. As second the trigger to load the new module.

But before we start to code, how we want to decouple the SideMenu(React .Component) from the Action? Here I take the idea from Google Material Design Components. They using foundations and adapter. To solve my question I just took the adapter idea:

...
export interface IAdapter {
  onMenu(name: string): void
}
...

As you can se, its just an interface. It is also our contract between SideMenu and Action.

And how look the test now?

...
  let model: Model, adapter: IAdapter;

  beforeEach(() => {
    attachToSpy = jest.fn();
    model = new Model();
    model.isOpen = true;
    model.pageNames = ['settings'];
    model.translation = {
      settings: 'Settings menu'
    };
    model.isActive = {
      settings: false
    };
    model.url = {
      settings: './settings/'
    };
    adapter = {
      onMenu: jest.fn()
    };
  });

  it('Manage menu clicks', async () => {
    const instance: RenderResult = render(<SideMenu model={model} adapter={adapter} />);

    const event: React.MouseEvent<HTMLAnchorElement> | MouseEvent = new MouseEvent('click', {
      bubbles: true,
      cancelable: true
    });
    event.stopPropagation = jest.fn();

    const settingsMenuElement: HTMLElement = await instance.findByTestId('settings');
    fireEvent(settingsMenuElement, event);

    expect(event.stopPropagation).toHaveBeenCalled();
    expect(event.defaultPrevented).toBeTruthy(); // preventDefault() is not mock able
    expect(adapter.onMenu).toHaveBeenCalledWith('settings');
  });
...

Here we are. A "Font side" test, without any business rules. Just visual rules. So the SideMenu don't need to know:

• How page is changed.
• How module is loaded.
• How url is actualized.

The second part is just missing: "...,than change page."

So lets do that in the Action:

...
export default class Action {
  router: Router;
  routerRegistry: RouterRegistry;

  constructor(routerRegistry: RouterRegistry) {
    this.router = router;
    this.routerRegistry = routerRegistry;
  }

  get adapter(): IAdapter {
    return {
      onMenu: this.switchPage.bind(this)
    };
  }

  protected switchPage(name: string): void {
    const page: IPageData = this.routerRegistry.getPages()[name];
    this.router.changePage(page);
  }
}
...

...and the test...

...
describe('Application.Action', () => {
  let router: Router, routerRegistry: RouterRegistry;

  beforeEach(() => {
    router = jest.genMockFromModule<Router>('../Router/Router');
    routerRegistry = jest.genMockFromModule<RouterRegistry>('../Router/Registry');
  });

  it('Change page', () => {
    const action = new Action(router, routerRegistry);
    const page: IPageData = {
      depth: 0,
      name: 'name',
      url: 'url'
    };
    routerRegistry.getPages = jest.fn().mockReturnValue({name: page});
    router.changePage = jest.fn();

    action.adapter.onMenu('name');
    expect(router.changePage).toHaveBeenCalledWith(page);
  });
});
...

The loading of the new module, I also decoupled here via a state observer. That seems now like that:

1. Router change state of Router State.
2. Router state calls callback of another Action. => Business rule: >> 3) Is page changed, than load module.
3. Action calls ModuleLoader

At least implement the rule >> 3):

...

  export interface IAdapter {
    onPageChanged(oldValue: IPageData, newValue: IPageData): void;
    onMenu(name: string): void
  }

...

  get adapter(): IAdapter {
    return {
      onPageChanged: this.loadModule.bind(this),
      onMenu: this.switchPage.bind(this)
    };
  }

  protected loadModule(oldValue: IPageData, newValue: IPageData) {
    this.moduleLoader.loadModule((newValue as IModulePageData).module).then();
  }
...

...and the test...

...
  it('Load module on page change', () => {
    const action = new Action(menuOpenState, router, routerRegistry, moduleLoader);
    const page: IModulePageData = {
      module: './New/Module',
      depth: 0,
      name: 'name',
      url: 'url'
    };
    moduleLoader.loadModule = jest.fn().mockResolvedValue(undefined);

    action.adapter.onPageChanged(page, page);
    expect(moduleLoader.loadModule).toHaveBeenCalledWith('./New/Module');
  });
...

Goal reached! We have decoupled, view from business and business from processing. That is now really more clean code.

What we learned this time?
Clean code have nothing to do with code formatting. 😉

Thanks for reading this very long article.

PS: Maybe you saw my posible mistake with .then();? ... but that is another story.