How to improve this: Hell callback?

8

I'm finishing the development of an ad system, the system uses the redis server to save data and display campaigns directly from ram, since the pathology of these systems requires a lot of disk consumption. At every x time I need to get the values scattered through collections on my redis server and save on two relational tables on my mysql server. However, I'm too young for asynchronous programming and do not master the promises, although the code below seems to work fine. I would not want to leave you with this wording, because it becomes difficult to maintain the code.

This routine inserts the cost of each campaign, including user connections, campaign impressions, and ad clicks on affiliate sites. In other words, I need a loop for each zone created by the editors, adding the number of connections in those zones, then add to that collection the impressions and clicks of each campaign and send on 15-second intervals to the mysql server. / p>

SalvarStatsCustosCampanha = function(){
  cliente_redis.smembers("campanhas_disponiveis",function(err,campanhas){
    var multi                                    = cliente_redis.multi();
    for(var i= 0; i<campanhas.length;i++){
      multi.smembers("zona_membros_campanhas:"+campanhas[i]);
    }
    multi.exec(function(err,zonas){
      async.forEachOf(zonas,function(lista,indice,callback){
        var desconectados                        = 0;
        var conectados                           = 0;
        async.each(lista,function(zona,fn){
          desconectados                         += (typeof(global.desconectados_zona[zona]) != 'undefined')? global.desconectados_zona[zona] : 0;
          conectados                            += (typeof(global.conectados_zona[zona])    != 'undefined')? global.conectados_zona[zona]    : 0;
          fn();
        },function(err){
          cliente_redis.hmget("campanha:"+campanhas[indice],['id_usuario','cliques','impressoes','custo'],function(err,info){
            let campanha                         = {};
            campanha.id_anunciante               = info[0];
            campanha.id_campanha                 = campanhas[indice];
            campanha.conectados                  = desconectados;
            campanha.impressoes                  = (info[2] == null)? 0 : info[2];
            campanha.cliques                     = (info[1] == null)? 0 : info[1];
            campanha.custo                       = (info[3] == null)? 0 : info[3];
            if(campanha.cliques                 == 0){
              campanha.ctr                       = 0;
              campanha.ctr                       = campanha.ctr.toFixed(3);
            }else campanha.ctr                   = (parseFloat(campanha.cliques) / parseFloat(campanha.conectados)).toFixed(3);
            campanha.data                        = moment().format("YYYY-MM-DD");
            campanha.fechamento                  = moment().format();
            var data                             = {};
            data.campanha                        = campanha;
            data.data                            = moment().format("YYYY-MM-DD");
            fnCampanha.GetCustoCampanhaDia(data,function(data,custo){
              CampanhaSangria                    = function(campanha){
                cliente_redis.hmset("campanha:"+campanha.id_campanha,{"cliques": 0,"custo" : 0,"impressoes": 0});
              }
              if(typeof(custo) != 'undefined'){
                data.campanha.id_custo           = custo.id_custo;
                fnCustos.AlterarCustos(data.campanha,CampanhaSangria);
              }else{
                fnCustos.AdicionarCustos(data.campanha,CampanhaSangria);
              }
            });
          });
        });
      });
    });
  });
}

The same logic is used for the campaign zones responsible for linking ads on affiliate sites, only the number of impressions is not computed because the zone receives by real-time socket dozens of campaigns so I only effective accounting of impressions for campaigns.

SalvarStatsRendimentosZona = function(){
  cliente_redis.smembers('zonas_disponiveis',function(err,zonas){
    if(zonas.length > 0){
      Falha                                       = function(){
        console.log("O processo de update de rendimentos falhou");
      }
      Sucesso                                     = function(data,zona){
        cliente_redis.hmget("zona:"+data.zona.id_zona,['cliques','rendimentos'],function(err,cliques){
          if(cliques != null){
            let rendimento                        = {};
            rendimento.conectados                 = (!isNaN(global.desconectados_zona[zona.id_zona]))? global.desconectados_zona[zona.id_zona]: 0;
            /* Se não houver conexões sobre a zona no dia corrente não há necessidade de alterar ou inserir apenas ignoramos
            esse registro até que alguma conexão exista */
            if(parseInt(rendimento.conectados) > 0){
              rendimento.id_zona                  = zona.id_zona;
              rendimento.id_publicador            = zona.id_publicador;
              rendimento.cliques                  = cliques[0];
              if(rendimento.cliques              == 0){
                rendimento.ctr                    = 0;
                rendimento.ctr                    = rendimento.ctr.toFixed(3);
              }else rendimento.ctr                = (parseFloat(rendimento.cliques) / parseFloat(rendimento.conectados)).toFixed(3);
              rendimento.ganhos                   = parseFloat(cliques[1]).toFixed(3);
              rendimento.fechamento               = moment().format();
              data.rendimento                     = rendimento;
              data.data                           = moment().format("YYYY-MM-DD");
              fnRendimento.GetRendimentosZonaDia(data,function(data,rendimentos){
                ZonaSangria                       = function(){
                  cliente_redis.hmset("zona:"+data.zona.id_zona,{"cliques": 0,"rendimentos" : 0});
                  cliente_redis.hmset("stats:zonas:"+data.zona.id_zona,{"desconectados" : 0});
                  global.desconectados_zona[data.zona.id_zona] = 0;
                }
                data.rendimento.data              = data.data;
                if(typeof(rendimentos)           != 'undefined'){
                  data.rendimento.id_rendimento   = rendimentos.id_rendimento;
                  fnRendimento.AlterarRendimento(data.rendimento);
                  ZonaSangria();
                }else{
                  fnRendimento.AdicionarRendimentos(data.rendimento);
                  ZonaSangria();
                }
              });
            }
          }
        });
      }
      zonas.forEach(function(id_zona){
        var data                              = {};
        data.zona                             = {};
        data.debug                            = 'zonas disponiveis';
        data.zona.id_zona                     = id_zona;
        data.sucesso                          = Sucesso;
        data.sucesso_redis                    = Sucesso;
        data.falha                            = Falha;
        fnZona.GetZonaById(data);
      });
    }
  });
}

Finally, I have to run these two routines simultaneously to avoid clearing the global script variable that keeps the number of visitors disconnected before saving to my database, since after saving in both tables I assign zero to her in each campaign zone. Losing the disconnections would risk the statistical calculation of the application. I need to prevent the socket disconnect event from confusing the results being saved to the database when my setInterval runs because it is a global scopo variable, so I used the default new async await by changing the version of my node server .js from 6 to 8, to simplify coding and run functions simultaneously. How can I improve this code? I would like something upright that would allow me to better maintain the code in the future.

setInterval(function(){
  async function ExecuteActions(){
    await SalvarStatsCustosCampanha();
    await SalvarStatsRendimentosZona();
  }
  ExecuteActions();
},configs.ttl_save_stats);

I know the question is comprehensive and requires time for analysis plus any opinion would be valid!

    
asked by anonymous 19.09.2018 / 17:09

1 answer

2

First let's solve the problem of your setInterval . To ensure that there are no two concurrent runs of the same code, I suggest that you change to setTimeout and restart the counter after running the root function (prinicpal) as follows:

const execucaoRaiz = async () => {
  try {
    // Garantirá que as duas funções executarão em paralelo
    const promessas = [
      salvarStatsCustosCampanha(),
      salvarStatsRendimentosZona(),
    ];

    // O código só prosseguirá quando as duas execuções forem completadas
    await Promise.all(promessas);

    // Eventual código adicional após a execução das duas funções principais
    // ...
  } catch(e) {
    console.error(e);
  }

  // Garante que o timer será reiniciado mesmo que o código ocasione algum erro
  iniciarTimer();
};

const iniciarTimer = async () => {
  setTimeout(execucaoRaiz, configs.ttl_save_stats);
}

// Executará a primeira vez após o tempo determinado
iniciarTimer();

Soon after this change, I suggest you use promises in the redis case. I do not know exactly how the module execution is, but looking in your code I believe that the callback structure is the one that is agreed by the node community ( function callback(erro, resultado) {} ). That way you can use the util.promisify function to transform them on promises:

const { promisify } = require('utils');

// ...

const smembers = promisify(cliente_redis.smembers);

const campanhas = await smembers('campanhas_disponiveis');

// ...

Finally turn your functions into async so you can use await inside them:

const salvarStatsCustosCampanha = async () => {
  // Lógica da função
}

It will be imperative that you break the code into more functions to make it easier to understand in future maintenance.

In my examples above I used the AirBNB Style Guide as a base. Following it will leave your code cleaner and readable and also apply some good practices to it.

I have outlined in general lines because your code is not executable, so it becomes difficult to rewrite it without the possibility of testing the result.

    
28.09.2018 / 05:58