D
D
Dvorak2016-09-05 23:04:46
JavaScript
Dvorak, 2016-09-05 23:04:46

How to comb the code?

Hello. I am writing a bot for telegram. At the moment the code looks something like this:

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;
    };
  });
});

+ in the last case there will be another switch -> case.
The code is not very beautiful, I would like something like:
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;
    };

If I try to implement this, then in the first case I will have to pass at least 5 arguments to the function. And it will never make this code more readable. How to be?

Answer the question

In order to leave comments, you need to log in

2 answer(s)
F
Faliah, 2016-09-06
@Faliah

You can simplify anything, just by looking at the problem from the other side. There are several questions about the code: 1) You pass two strings to
the function and code lang="javascript">'language' (in red rectangles). Is it possible to get rid of them, because, in fact, they are constants? The same goes for their counterparts in red rounded rectangles. If it is necessary to pass to each function , then, in my opinion, it would be useful to make a wrapperredis.hmget'level'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);
    
  });
});

Generally speaking, the logic of forming arrays with arguments can also be simplified and made uniform, or the app.sendMessage and redis.hmset functions can be made less polymorphic - different calls pass a different number of arguments and sometimes in a different order. It's hard to understand

V
Vitaly, 2016-09-12
@vshvydky

I'm wildly sorry, but it just annoys me that errors are completely ignored? This is the devil's machine.
And instead of case, I like to use object methods. Maybe you will be comfortable.
Something like
Obj ={
'Name name': function(param){...}
}
Obj['name name'](msg) and put all your cases into an object?

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question