Refactor Jquery .text () function

Question

I'm really concerned about clean programming and nice readable code.

My goal is to change string (text), when user clicks on various products on the webpage. I noticed that my code is repeating and looks very simple (not professional). The most appropriate option would be to refactor my code and make it look better.

var text = $('.inner-container h1').text();
var changeString = $('.inner-container h1');

$('.ottogi').click(function() {
    if (text == 'Ottogi') {
        $(changeString).text('Ottogi');
    }
});
 $('.sajo').click(function() {
     if (text == 'Ottogi') {
        $(changeString).text('Sajo Hapyo');
    }
 });
  $('.natura').click(function(){
     if (text == 'Ottogi') {
        $(changeString).text('Natura Bogata');
    }
 });
$('.maloo').click(function(){
     if (text == 'Ottogi') {
        $(changeString).text('ТОО Малу');
    }
 });
$('.dongush').click(function(){
     if (text == 'Ottogi') {
        $(changeString).text('Dongsuh');
    }
 });
 $('.may').click(function(){
     if (text == 'Ottogi') {
        $(changeString).text('ООО Май');
    }
 });

Answer 1

You could reduce duplication by using an object mapping the selectors (class names in this case) to their responses

const selectorToResponse = {
  '.ottogi': 'Ottogi',
  '.sajo': 'Sajo Hapyo',
  '.natura': 'Natura Bogata',
  '.maloo': 'TOO Maлy',
  '.dongush': 'Dongsuh',
  '.may': 'OOO Maй'
}

Note, that for the first case (.ottogi) you are changing the text to Ottogi if it is Ottogi which has no change, so it can be removed.

const selectorToResponse = {
  '.sajo': 'Sajo Hapyo',
  '.natura': 'Natura Bogata',
  '.maloo': 'TOO Maлy',
  '.dongush': 'Dongsuh',
  '.may': 'OOO Maй'
}

From here you could loop over the object entries using a for...in loop and define the event handlers. This way you only have to add further entries to selectorToResponse instead of duplicating.

const selectorToReponse = {
  '.ottogi': 'Ottogi',
  '.sajo': 'Sajo Hapyo',
  '.natura': 'Natura Bogata',
  '.maloo': 'TOO Maлy',
  '.dongush': 'Dongsuh',
  '.may': 'OOO Maй'
}

// Prefixing with $ to denote that is an element, not text etc
const $header = $('.inner-container h1')

for (const selector in selectorToResponse) {
  $(selector).click = function () {
    // using $header.text() inside function so that is current header text
    // `text` never changed in your example
    if ($header.text() === 'Ottogi') { // using === instead of ==
      const response = selectorToResponse[selector]

      $header.text(response)
    }
  }
}

Notes

Above uses strict equality comparison (===) instead of abstract equality comparison (==). https://developer.mozilla.org/en/docs/Web/JavaScript/Equality_comparisons_and_sameness

Solution uses const, an ES6 feature. The benefits are that selectors scope is confined to the for loop and that the variable identifier cannot be reassigned.

Depending on your desired browser compatibility it may or may not be available.

As @IcePickle pointed out, using var in the for...in loop would mean that selector is always .may in the click handlers due to var having different socping rules. See Creating closures in loops - A common mistake for a detailed explanation.

Using var and another closure:

var selectorToReponse = {
  '.ottogi': 'Ottogi',
  '.sajo': 'Sajo Hapyo',
  '.natura': 'Natura Bogata',
  '.maloo': 'TOO Maлy',
  '.dongush': 'Dongsuh',
  '.may': 'OOO Maй'
}

var $header = $('.inner-container h1')

for (var selector in selectorToResponse) {
  $(selector).click = (function (selector) {
    return function () {
      if ($header.text() === 'Ottogi') {
        var response = selectorToResponse[selector]

        $header.text(response)
      }
    }
  })(selector)
}