Open-gesloten principe. Moet ik het ooit gebruiken?

Dus wat betekent het echt?

We zijn een ontwikkelprobleem tegengekomen dat ons heeft laten zien wat het open-gesloten principe eigenlijk inhoudt. In een van onze webapplicaties hadden we een formulier met (onder andere) twee secties:

• vraagkanalen
• dynamische filters

Gebruikers kunnen zoveel filters toevoegen als ze willen, maar er zijn enkele regels - de beschikbaarheid van filters hangt af van de gekozen kanalen.

Vraagkanalen: ADUITWISSELING, KOPBIDDING, RESERVERING, ANDERE Dynamische filters(dimensies): website, advertentieeenheid, geo, creatiefgrootte, apparaat

Dit artikel gaat vooral over code refactor, dus er zullen hieronder veel codefragmenten staan. Ik heb geprobeerd om het te verminderen, maar sommige hoeveelheid code is nodig om te laten zien code refactoring. Je hoeft niet elk klein deel van de code te begrijpen om de hoofdgedachte te begrijpen.

De eerste implementatie van het probleem was eenvoudig:

klasse ResearchFormStateUpdater {
  update () {
    (...)
    this._updateDynamicFilters();
  }

  _updateDynamicFilters () {
    $('.dynamisch-filter').each((_, filter) => {
      $(filter).trigger('dynamicFilter:disableWebsites', this._shouldDisableWebsitesFields());
    });
  }

  _shouldDisableWebsitesFields () {
    return this._shouldDisableFields(ResearchFormateUpdater.WEBSITE_DISABLING_DEMAND_CHANNELS);
  }

  _shouldDisableFields (disablingDemandChannels) {
    // is een van disablingDemandChannels aangevinkt?
  }
}

ResearchFormStateUpdater.WEBSITE_DISABLING_DEMAND_CHANNELS = ['header_bidding', 'reservation', 'other'];

klasse OnderzoekDynamischFilter {
  _setDynamicFilterDisableWebsitesEvent () {
    $(this._getBody()).on('dynamicFilter:disableWebsites', (event, shouldDisableWebsites) => {
      // website filters uitschakelen
    });
  }
}

Zoals je kunt zien, is de websitefilter niet beschikbaar voor HEADERBIDDING, RESERVERING en ANDERE kanalen, dus het is alleen beschikbaar voor ADUITWISSELkanaal.

Het laatste wat je over code kunt zeggen is dat het permanent of statisch is. We krijgen dus meer verzoeken van onze klant waardoor deze klassen groter en complexer worden.

Ontwikkeling van functies

• Nog een kanaal toevoegen - EBDA (Website filter moet onbeschikbaar zijn terwijl EBDA is gekozen):

  • uitbreiden UITSCHAKELEN_VRAAG_KANALEN door EBDA vraagkanaal

  • veel naamswijzigingen - in de eerste implementatie hebben we website gespecificeerd in de namen van methoden en constanten. Bijvoorbeeld:

    • isWebsitesDimensionDisabled naar _areFormStateDimensionsDisabled
    • WEBSITE_UITSCHAKELEN_VRAAGKANALEN naar UITSCHAKELEN_VRAAG_KANALEN

Spoiler alert -> wanneer een component open is voor wijzigingen, zal er in de toekomst veel van naam veranderen. In de volgende stappen zullen we hier geen aandacht aan besteden.

• Nog een filter toevoegen voor 'Product (Product filterbeschikbaarheidsschema is hetzelfde als Website)

  • OnderzoekDynamischFilter klasse moet controleren op nog een dimensie tijdens het uitschakelen/inschakelen van velden

• Laten we groter gaan en een switcher toevoegen boven kanalen -> 'Bron'. Alle vraagkanalen die we tot nu toe hadden, staan in de bron Ad Manager. De nieuwe bron - SSP - heeft geen vraagkanalen en het enige beschikbare filter is website.

  • Regels:

    • Er zijn twee bronstatussen: Ad Manager, SSP.
    • Al onze vraagkanalen zijn alleen beschikbaar voor Ad Manager-bronnen.
    • Er zijn geen vraagkanalen voor SSP-bron
    • Website' is het enige beschikbare filter voor SSP-bron.

  • Implementatie:

    • Wanneer 'SSP' wordt gekozen:

      • Schakel vraagkanalen uit.
      • trigger "dynamischFilter:websitesenProducten uitschakelen <- beide inschakelen
      • trigger "dynamischFilter:uitschakelennietSspOpties

    • Wanneer Ad Manager is aangevinkt:

      • trigger "dynamischFilter:websitesenProducten uitschakelen <- controleer weer ingeschakeld of uitgeschakeld

• Nog een filter toevoegen voor 'Platform

  • Regels:

    • Platform is alleen beschikbaar als de bron SSP is

  • Moeilijkheidsgraad:

    • Nu hebben we 'Website', die beschikbaar is voor het AD_EXCHANGE-kanaal van Ad Manager en voor Ssp, en we hebben 'Platform', dat beschikbaar is voor Ssp maar niet voor Ad Manager.
    • Schakelen tussen de staat van het formulier kan echt lastig en verwarrend worden

Implementatie met nieuwe functionaliteit:

Ik presenteer het volgende fragment voornamelijk om de complexiteit van de code te laten zien. Voel je vrij om het verborgen te laten.

klasse OnderzoekFormulierUpdater {
  update () {
    (...)
    this._triggerCallbacks();
  }

  _triggerCallbacks () {
    // kies callbacks afhankelijk van bron
  }

  _adManagerSourceCallbacks () {
    (...)
    this._enableDemandChannels(ResearchFormStateUpdater.AD_MANAGER_DEMAND_CHANNELS);
    this._updateDefaultStateOfDynamicFilters();
    this._updateAdManagerDynamicFilters();
  }

  _sspSourceCallbacks () {
    (...)
    this._removeDemandChannelsActiveClassAndDisable(ResearchFormStateUpdater.AD_MANAGER_DEMAND_CHANNELS);
    this._updateDefaultStateOfDynamicFilters();
  }

  _updateDefaultStateOfDynamicFilters () {
    $('.dynamic-filter').each((_, filter) => {
      $(filter).trigger('dynamicFilter:enableSspFilters', this.isSourceSsp);
    });
  }

  _updateAdManagerDynamicFilters () {
    $('.dynamisch-filter').each((_, filter) => {
      $(filter).trigger('dynamicFilter:disableWebsitesAndProducts', this._areFormStateDimensionsDisabled() && ! this.isSourceSsp);
    });
  }

  _shouldDisableFields (disablingDemandChannels) {
    // is een van disablingDemandChannels is aangevinkt
  }
}

ResearchFormStateUpdater.AD_MANAGER_DISABLING_DEMAND_CHANNELS = ['header_bidding', 'reservation', 'other', 'ebda'];

klasse OnderzoekDynamischFilter {
  // Ik heb deze twee methoden niet vereenvoudigd om de huidige complexiteit van de implementatie aan te tonen.

  _setDefaultDynamicFiltersToggleEvent () {
    $(this._getBody()).on('dynamicFilter:enableSspFilters', (event, shouldEnableSspOptions) => {
      this._setDefaultFiltersOptionDisabledState(shouldEnableSspOptions);

      const selectedFilterDimension = this._getFiltersDimension().find('option:selected').val();
      if (selectedFilterDimension === 'website') {
        this._toggleChosenFilterDisabledState(false);
      } anders, if (selectedFilterDimension == "platform") {
        this._toggleChosenFilterDisabledState(!shouldEnableSspOptions);
      } anders {
        this._toggleChosenFilterDisabledState(shouldEnableSspOptions);
      }
    });
  }

  _setDynamicFilterDisableWebsitesAndProductsEvent () {
    $(this._getBody()).on('dynamicFilter:disableWebsitesAndProducts', (event, shouldDisableWebsitesAndProducts) => {
      const selectedFilterDimension = this._getFiltersDimension().find('option:selected').val();
      if ($.inArray(selectedFilterDimension, ['website', 'product']) >= 0) {
        this._toggleChosenFilterDisabledState(shouldDisableWebsitesAndProducts);
      }
      this._setMethodSelectWebsiteAndProductOptionDisabledState(shouldDisableWebsitesAndProducts);
    });
  }

  _toggleNonSspFilters (dimensionSelect, shouldDisable) {
    $.each(ResearchDynamicFilter.NON_SSP_FILTERS_OPTIONS, (_, option) => {
      // filterstatus wisselen afhankelijk van 'shouldDisable'.
    });
  }
}

ResearchDynamicFilter.NON_SSP_FILTERS_OPTIONS = ['ad_unit', 'creative_size', 'geo', 'device', 'product'];

We gebruiken nog steeds een aantal wisselknop mechanisme. Het is echt moeilijk om 4 hendels om te zetten en de verwachte status te bereiken en nu moet DynamicFilter weten welke dimensies niet voor ssp bron zijn.

We hebben ResearchFormStateUpdater, waarom zou die niet de leiding hebben?

Definitief verzoek

Nog een filter toevoegen voor 'Yield partner

Dat is precies het moment waarop we besloten om die klassen te refactoren. Kanalen en filters die worden geanalyseerd zijn slechts een klein deel van het probleem. Er zijn hier meerdere formulieronderdelen en ze hebben allemaal hetzelfde probleem. Onze refactor zou de noodzaak voor het veranderen van methoden in die klassen *moeten* neutraliseren om nieuwe kanalen of dimensies toe te voegen.

In het volgende fragment heb ik de hoofdklassen bijna hetzelfde gelaten als in onze productiecode om te laten zien hoe eenvoudig ze nu te begrijpen zijn.

klasse OnderzoekFormulierUpdater {
  update () {
    (...)
    this._updateDynamicFilters();
  }

  _updateDynamicFilters () {
    this._toggleAllDynamicFiltersState(this._dynamicFiltersDimensionsToBeDisabled());
  }

  _dynamicFiltersDimensionsToBeDisabled () {
    if (this.isSourceSsp) { return ResearchFormateUpdater.NO_SSP_FILTERS; }

    var disabledFilters = ResearchFormateUpdater.ONLY_SSP_FILTERS;
    if (this.areDemandChannelsExceptAdxSelected) {
      disabledFilters = disabledFilters.concat(ResearchFormateUpdater.ONLY_ADX_FILTERS);
    }
    return disabledFilters;
  }

  _toggleAllDynamicFiltersState (disabledFilters) {
    $('.dynamisch-filter').each((_, filter) => {
      this._toggleDynamicFilterState(filter, disabledFilters);
    });
  }

  _toggleDynamicFilterState (dynamischFilter, uitgeschakeldFilters) {
    $(dynamicFilter).trigger('dynamicFilter:toggleDynamicFilters', disabledFilters);
  }
}

ResearchFormStateUpdater.NO_SSP_FILTERS = ['ad_unit', 'creative_size', 'geo', 'device', 'product'];

ResearchFormStateUpdater.ONLY_SSP_FILTERS = ['platform'];

ResearchFormStateUpdater.ONLY_ADX_FILTERS = ['website', 'product'];

klasse onderzoekdynamisch filter {
  _setDynamicFiltersToggleEvent () {
    $(this._getBody()).on('dynamicFilter:toggleDynamicFilters', (event, disabledFilters) => {
      this._disableFilters(disabledFilters.split(','));
      this._enableFilters(disabledFilters.split(','));
    });
  }

  _disableFilters (filtersToDisable) {
    // filtersToDisable uitschakelen
  }

  _enableFilters (filtersToDisable) {
    const filtersToEnable = $(ResearchDynamicFilter.ALL_FILTERS).not(filtersToDisable).get();
    // filters inschakelenToEnable
  }
}

ResearchDynamicFilter.ALL_FILTERS = ['website', 'ad_unit', 'creative_size', 'geo', 'device', 'product', 'platform'];

Het is ons gelukt! Is het gelukt?

Het enige dat 'ResearchDynamicFilter' nu hoeft te weten is een lijst van alle filters - dat lijkt redelijk. De rest van de logica en controle komt van boven - een aantal hogere methoden en constanten.

Laten we onze nieuwe structuur eens uitproberen door een filter toe te voegen voor 'Yield_partner':

klasse ResearchFormStateUpdater {
  _dynamicFiltersDimensionsToBeDisabled () {
    (...)
    if (this.areDemandChannelsExceptEbdaSelected) {
      disabledFilters = disabledFilters.concat(ResearchFormateUpdater.ONLY_EBDA_FILTERS);
    }
    return disabledFilters;
  }
}

ResearchFormStateUpdater.NO_SSP_FILTERS = [(...), 'yield_partner'];

ResearchFormStateUpdater.ONLY_EBDA_FILTERS = [(...), "yield_partner"];

ResearchDynamicFilter.ALL_FILTERS = [(...), "yield_partner"];

Zoals je kunt zien, gaat het om het toevoegen van enkele waarden aan constanten en enkele aanvullende voorwaarden.

Dankzij het 'open-gesloten principe' kunnen we de bedrijfslogica van een formulier wijzigen door alleen wat waarden en voorwaarden op een hoger abstractieniveau toe te voegen. We hoeven niet in het component te gaan en iets te veranderen. Deze refactor heeft het hele formulier beïnvloed en er waren meer onderdelen die nu allemaal voldoen aan het open-gesloten principe.

We hebben de hoeveelheid code niet verminderd - sterker nog, we hebben het zelfs vergroot (voor/na):

• ResearchFormStateUpdater - 211/282 lijnen
• OnderzoekDynamischFilter - 267/256 lijnen

Het gaat allemaal om de verzameling in constanten -> het is nu onze openbare interface, onze console om het proces te besturen zonder tientallen switchers.

Lees ook:

• Hoe schrijf je een goede code?
• Vuekalender. Een nieuw project van Codest gebaseerd op Vue.js
• Wat is Ruby on Jets en hoe bouw je er een app mee?