Как причесать код?

Question

[Dvorak]

Здравствуйте. Пишу бота для телеграма. На данный момент код выглядит примерно так:

app.on('message', (msg) => {
  const telegramID = msg.from.id,
        text = msg.text;

  redis.hmget(`id${telegramID}`, 'level', 'language', (err, res) => {
    switch (res[0]) {
      case 'language':
        app.sendMessage(telegramID, message[text].languageGood);
        redis.hmset(`id${telegramID}`, 'level', 'user_name', 'language', msg.text);
      break;

      case 'user_name':
        app.sendMessage(telegramID, message[res[1]].nameGood, options);
        redis.hmset(`id${telegramID}`, 'user_name', text, 'level', 'change_services');
      break;

      case 'change_services':
        if (text == 'some text') {
          app.sendMessage(telegramID, message[res[1]].flpPhone);
          redis.hmset(`id${telegramID}`, 'level', 'flp', 'sublevel', 'phone');
        } else if (text == 'another text') {
          redis.hmset(`id${telegramID}`, 'level', 'ooo');
        } else if (text == `other`) {
          redis.hmset(`id${telegramID}`, 'level', 'reg')
        } else {
          app.sendMessage(telegramID, message[res[1]].changeServicesBad);
        }
      break;
    };
  });
});

+ в последнем case будет ещё один switch -> case.

Код не очень красивый, хотелось бы что-то типа:

app.on('message', (msg) => {
  const telegramID = msg.from.id,
        text = msg.text;

  redis.hmget(`id${telegramID}`, 'level', 'language', (err, res) => {
    switch (res[0]) {
      case 'language':
        func1();
      break;

      case 'user_name':
        func2();
      break;

      case 'change_services':
        func3();
      break;
    };

Если попытаться это реализовать, то в первом же case мне в функцию придётся передавать минимум 5 аргументов. И это ни разу не сделает этот код читабельнее. Как быть?

Comments

[napa3um]

Передавайте 5 аргументов. Это нормально.

[Dvorak]

napa3um: считается же, что больше 3 - не комильфо. Я слышал это много раз. Даже препод в универе мне это говорил

[napa3um]

Андрей: постарайтесь перейти от количественных оценок к качественным. Если аргументы понятны и логичны в том месте, в котором они возникают, то, вероятно, декомпозиция на функции произведена уместно.

[S3Ga]

передавайте объект

[Максим]

Андрей: так сделайте объект (типа params) и в нем уже указывайте нужные свойства. Тогда нет смысла запоминать порядок следования аргументов.

Answer 1

[Faliah]

Упростить можно всё, что угодно, просто взшлянув на проблему с другой стороны. Есть несколько вопросов по коду:


1) В функцию redis.hmget вы передаёте две строки 'level' и code lang="javascript">'language' (в красных прямоугольниках). Нельзя ли избавиться от них, ведь, по сути это константы? То же самое касается их аналогов в красных прямоугольниках со скруглёнными уголками. Если в каждую функцию redis.hmset нужно обязательно передать 'level', то, на мой взгляд полезно было бы сделать обёртку

function memoizeArguments() {
  
  const memoizeArgs = Array.from(arguments);
  
  return function(fun) {
    return function() {
      fun.apply(null, memoizeArgs.concat(Array.from(arguments).join()));
    }
  }
  
}

app.on('message', msg => {
  
  const telegramID = msg.from.id,
        encodedID = `id${telegramID}`,
        text = msg.text;

  // Сохраняем аргументы, чтобы упростить сигнатуры вызовов, создаём обертки с закэшированными аргументами
  const withId = memoizeArguments(telegramID);
  const appSendMessage = withId(app.sendMessage);
  
  const withEncodedIdAndLevel = memoizeArguments(encodedID, 'level');
  const redisHmSet = withIdAndLevel(redis.hmset);
  
  

  redis.hmget(encodedID, 'level', 'language', (err, res) => {
    
    let sendMessageArgs = null;
    let redisHmSetArgs = null;
    
    switch (res[0]) {
      case 'language':
        sendMessageArgs = message[text].languageGood;
        redisHmSetArgs = ['user_name', 'language', msg.text];
      break;

      case 'user_name':
        sendMessageArgs = [message[res[1]].nameGood, options];
        redisHmSetArgs = ['user_name', text, 'change_services']; // тут парамет 'level' передастся 2м аругментом, а не 4м
      break;

      case 'change_services':
        
        switch(text) {
            
          case 'some text':
            sendMessageArgs = message[res[1]].flpPhone;
            redisHmSetArgs = ['flp', 'sublevel', 'phone'];
            break;

          case 'another text':
            redisHmSetArgs = 'ooo';
            break;  
            
          case 'other':
            redisHmSetArgs = 'reg';
            break; 
            
          default:
            sendMessageArgs = message[res[1]].changeServicesBad;
            break;
        }
       
      break;
    }

    if (sendMessageArgs) appSendMessage(sendMessageArgs);
    if (redisHmSetArgs) redisHmSet(redisHmSetArgs);
    
  });
});

Вообще говоря логику фомирования массивов с аргументами тоже можно упростить и сделать единообразной, либо делать функции app.sendMessage и redis.hmset менее полиморфными - в разных вызовах передается разное число аргументов и иногда в разном порядке. Это сложно для понимания

Answer 2

[Виталий]

Я дико извиняюсь, но только меня напрягает, что ошибки полностью игнорируются? Это же шайтан машина будет.
А вместо case мне нравится использовать методы объектов. Может и вам удобным будет.
Что-то типа
Obj ={
'Name name': function(param){...}
}
Obj['name name'](msg) и вынести все ваши кейсы в объект?