2

Olá, gostaria da ajuda dos usuários para otimizar um script que fiz em node.js. Não entendo muito da linguagem e fiz um sistema bem amador que está um pouco lento.

var express = require('express')();
var http = require('http').Server(express);
var request = require('request');

var io = require('socket.io')(http);
var fs = require('fs');
var xml = require('xml2js');

var parser = new xml.Parser();

express.get('/', function(req, res){
    res.send('Dedicated server of Turn Network running!');
});

var tMusic;
var aTurn = {
    url: {
        history: '../../../Playlist/pgm/Montagem/MusicHistory.xml',
        current: '../../../Playlist/pgm/current.xml'
    },
    startServer: function(){
        aTurn.readData();

        setTimeout(function(){
            aTurn.updateData();
        }, 3000);
    },
    readData: function(){
        fs.readFile(aTurn.url.history, function(err, data){
            parser.parseString(data, function(err, result){
                tMusic = result['MusicHistory']['Item0'][0].$.Music;
            });
        });
        setTimeout(function(){
            aTurn.readData();
        }, 1000);
    },
    updateData: function(){
        fs.readFile(aTurn.url.history, function(err, data){
            parser.parseString(data, function(err, result){
                var current = result['MusicHistory']['Item0'][0].$.Music;
                var split = current.split(' - ');

                if(tMusic === current){
                }
                else if(tMusic == undefined || tMusic.length <= 0){
                    aTurn.emitData();
                }
                else{
                    aTurn.emitData();
                }
            });
        });
        setTimeout(function(){
            aTurn.updateData();
        }, 1000);
    },
    emitData: function(){
        function album(music){
            return new Promise(function(resolve, reject){
                request('http://source.localhost/provider/saveArt/'+music+'', function (error, response, result){
                    data = JSON.parse(result);
                    resolve(data['returnAlbum']);
                });
            });
        }
        fs.readFile(aTurn.url.current, function(err, data){
            parser.parseString(data, function(err, result){
                var music = result['Playlist']['Next'][0]['NextMusic'][0]['Music'][0]['Artist'] + ' - ' + result['Playlist']['Next'][0]['NextMusic'][0]['Music'][0]['Title'];
                var music = music.toString();

                var startTime = result['Playlist']['OnAir'][0]['CurIns'][0]['StartedTime'];
                var startTime = startTime.toString().split(' ');

                var endTime = result['Playlist']['Next'][0]['NextMusic'][0]['Music'][0].$.SchedTime;
                var endTime = endTime.split(' ');

                var nextArtist = result['Playlist']['Next'][0]['NextMusic'][0]['Music'][0]['Artist'];
                var nextMusic = result['Playlist']['Next'][0]['NextMusic'][0]['Music'][0]['Title'];

                album(music).then(function(nextAlbum){
                    io.emit('updateNext', {
                        'nextArtist': nextArtist.toString(), 
                        'nextMusic': nextMusic.toString(),
                        'nextAlbum': nextAlbum
                    });
                });
                io.emit('updateTime', {
                    'startTime': startTime[1], 
                    'endTime': endTime[1],
                    'maxTime': ''
                });
            });
        });
    }
}

io.on('connection', function(socket){
    aTurn.emitData();
});

http.listen(3000, function(){
    console.log('Client server running on port 3000!');
});

aTurn.startServer();

É um sistema que envia uma comunicação em socket sempre que tMusic mudar.

Alguém poderia me ajudar? Ficaria extremamente agradecido.

5
  • 1
    qual a necessidade de haver setTimeout(function(){ ... }, 3000) para então executar updateData() e readData() ?? Commented 25/05/2019 às 1:49
  • Para que antes de serem executadas, as mesmas já tenham lido os devidos arquivos. Pois sem o setTimeout estava dando undefined. Commented 13/06/2019 às 0:29
  • 4
    Nesse caso você deveria usar o callback do fs.readFile para executar o próximo passo somente depois que a leitura terminar. Usar o setTimeout aí é um gambiarra das fortes.
    – GBrandt
    Commented 14/06/2019 às 10:54
  • hahaha, foi bem na gambiarra mesmo, sou muito novato nisso. Commented 17/06/2019 às 21:56
  • comece usando fs.readFileSync e fs.writeFileSync para se livrar desse set timeout Commented 19/06/2019 às 13:35

1 Resposta 1

15
+150

Sugiro fortemente que você estude callbacks e procure entender realmente seu funcionamento. Seu código tem vários setTimeouts que não deveriam existir, e a sua lógica para ler e atualizar continuamente as coisas pode ser simplificada monstruosamente usando eles.

Corrigi o código com isso em mente, ficou assim:

var express = require('express')();
var http = require('http').Server(express);
var request = require('request');

var io = require('socket.io')(http);
var fs = require('fs');
var xml = require('xml2js');

var parser = new xml.Parser();

express.get('/', function(req, res){
    res.send('Dedicated server of Turn Network running!');
});

var tMusic;
var aTurn = {
    url: {
        history: '../../../Playlist/pgm/Montagem/MusicHistory.xml',
        current: '../../../Playlist/pgm/current.xml'
    },
    startServer: function(){
        aTurn.watchData(function(result) {
            aTurn.updateData(result);
        });
    },
    watchData: function(callback) {
        fs.watchFile(aTurn.url.history, function () {
            fs.readFile(aTurn.url.history, function(err, data){
                parser.parseString(data, function(err, result){
                    tMusic = result['MusicHistory']['Item0'][0].$.Music;

                    if (callback)
                        callback(result);
                });
            });
        });
    },
    updateData: function() {
        aTurn.emitData();
    },
    emitData: function(){
        function album(music){
            return new Promise(function(resolve, reject){
                request('http://source.localhost/provider/saveArt/' + music, function (error, response, result){
                    var data = JSON.parse(result);
                    resolve(data['returnAlbum']);
                });
            });
        }

        fs.readFile(aTurn.url.current, function(err, data){
            parser.parseString(data, function(err, result){
                var music = result['Playlist']['Next'][0]['NextMusic'][0]['Music'][0]['Artist'] + ' - ' + result['Playlist']['Next'][0]['NextMusic'][0]['Music'][0]['Title'];

                var startTime = result['Playlist']['OnAir'][0]['CurIns'][0]['StartedTime'];
                var startTime = startTime.toString().split(' ');

                var endTime = result['Playlist']['Next'][0]['NextMusic'][0]['Music'][0].$.SchedTime;
                var endTime = endTime.split(' ');

                var nextArtist = result['Playlist']['Next'][0]['NextMusic'][0]['Music'][0]['Artist'];
                var nextMusic = result['Playlist']['Next'][0]['NextMusic'][0]['Music'][0]['Title'];

                album(music).then(function(nextAlbum){
                    io.emit('updateNext', {
                        'nextArtist': nextArtist.toString(), 
                        'nextMusic': nextMusic.toString(),
                        'nextAlbum': nextAlbum
                    });
                });

                io.emit('updateTime', {
                    'startTime': startTime[1], 
                    'endTime': endTime[1],
                    'maxTime': ''
                });
            });
        });
    }
}

io.on('connection', function(socket) {
    aTurn.emitData();
});

http.listen(3000, function(){
    console.log('Client server running on port 3000!');
});

aTurn.startServer();

Claramente, como não sei o formato dos seus arquivos nem o resultado esperado do código, não tenho como testar que realmente funciona pro que você precisa, mas em termos de lógica assim fica melhor estruturado.


Correções feitas

Os problemas estavam basicamente todos em volta das funções readData e updateData. O primeiro:

1. setTimeout com objetivo de "faz um, depois faz o outro"

aTurn.readData();

setTimeout(function(){
    aTurn.updateData();
}, 3000);

Isso não se faz.

  • Primeiro problema: Não é estável. Se algum dia, por ventura, seu readData demorar mais que 3 segundos para executar, o updateData vai rodar sem que ele tenha completado.
  • Segundo problema: Para basicamente todo caso em que o timeout de fato funciona, você perde tempo esperando 3 segundos, quando na verdade o readData pode ter terminado em 5ms.

Solução

Adicione um callback à função readData que é chamado quando ela termina. Veja essa pergunta para mais informações sobre o que é o callback e onde ele é usado.

Nosso código refatorado fica:

aTurn.readData(function() {
    aTurn.updateData();
});

E no readData:

readData: function(callback) {
    fs.readFile(aTurn.url.history, function(err, data){
        parser.parseString(data, function(err, result){
            tMusic = result['MusicHistory']['Item0'][0].$.Music;

            if (callback)
                callback();
        });
    });
    setTimeout(function(){
        aTurn.readData();
    }, 1000);
},

Veja que o callback só é útil se você quiser rodar outras coisas além do updateData logo após o readData, e houver necessidade de rodar coisas diferentes para cada chamada do readData. Caso contrário, você pode só chamar o updateData do readData logo:

readData: function() {
    fs.readFile(aTurn.url.history, function(err, data){
        parser.parseString(data, function(err, result){
            tMusic = result['MusicHistory']['Item0'][0].$.Music;
            aTurn.updateData();
        });
    });
    setTimeout(function(){
        aTurn.readData();
    }, 1000);
},

2. setTimeout para ler arquivo de 1 em 1 segundo

readData: function() {
    fs.readFile(aTurn.url.history, function(err, data){
        // ...
    });
    setTimeout(function(){
        aTurn.readData();
    }, 1000);
},

Solução

Se eu entendi o que você queria, a ideia disso aí é ler o arquivo a cada x segundos e atualizar as coisas quando ele muda, correto?

Nesse caso, você pode usar a função fs.watchFile:

readData: function(callback) {
    fs.watchFile(aTurn.url.history, function () {
        fs.readFile(aTurn.url.history, function(err, data){
            // ...
        });
    });
},

Vale notar que o fs.watchFile pode não ser consistente em todas as plataformas, como a própria documentação declara. Leia com atenção a documentação dessa função.

3. Função updateData

    updateData: function(){
        fs.readFile(aTurn.url.history, function(err, data){
            parser.parseString(data, function(err, result){
                var current = result['MusicHistory']['Item0'][0].$.Music;
                var split = current.split(' - ');

                if(tMusic === current){
                }
                else if(tMusic == undefined || tMusic.length <= 0){
                    aTurn.emitData();
                }
                else{
                    aTurn.emitData();
                }
            });
        });
        setTimeout(function(){
            aTurn.updateData();
        }, 1000);
    },

Essa função inteira é redundante com as mudanças que fizemos no readData para chamar callback.

Veja que a leitura do arquivo pode sumir, já que lemos o arquivo no readData e podemos passar essa informação adiante via callback:

readData: function(callback) {
  // ...
  fs.readFile(aTurn.url.history, function(err, data) {
    parser.parseString(data, function(err, result){
        tMusic = result['MusicHistory']['Item0'][0].$.Music;

        if (callback)
            callback(result);
    });
  });
  // ...
}

Assim, usamos o data no updateData, sem ter que ler o arquivo de novo (que geralmente é uma operação bem cara, por sinal):

    startServer: function(){
        aTurn.watchData(function(result) {
            aTurn.updateData(result);
        });
    },
    // ...
    updateData: function(result) {
        var current = result['MusicHistory']['Item0'][0].$.Music;
        var split = current.split(' - ');

        if(tMusic === current){
        }
        else if(tMusic == undefined || tMusic.length <= 0){
            aTurn.emitData();
        }
        else {
            aTurn.emitData();
        }
    }

Agora temos mais um problema: esse if que não tem sentido nenhum.

veja que if (...) {}, sem código entre as {}, pode sempre ser reduzido a um código mais simples: ``. Pois é, nada. Some com ele.

Somado a isso, nosso tMusic é sempre atualizado logo antes do updateData, logo de toda forma nunca teríamos current != tMusic.

Temos:

if (tMusic == undefined || tMusic.length <= 0) {
    aTurn.emitData();
} else {
    aTurn.emitData();
}

Agora olha que legal, se tMusic == undefined || tMusic.length <= 0, então emitData. Se não, emitData também! Por que não?

Pois é, isso não é um if, é um aTurn.emitData(); com passos extras.

No fim, nossa função updateData vira isso:

    updateData: function() {
        aTurn.emitData();
    },

Ou seja, não precisa dela. Só chamar a emitData direto.

Nota: também não precisamos mais do setTimeout, porque a função é chamada toda vez que o readData é executado, e aquela função já repete!

Observações

  • Com a refatoração, fica claro que a variável tMusic não é usada e é inútil.
  • A função emitData sempre avança para a próxima música a partir da atual (que imagino estar no arquivo current), e não usa as informações do readData e updateData. Isso leva a crer que, na verdade, as funções readData e updateData são desnecessárias (na verdade, a updateData com certeza é, como já vimos).
1
  • 5
    Muito obrigado amigo, de coração! Além de ter dado certo, aprendi bastante com usa explicação! Abraços. Commented 17/06/2019 às 22:09

Você deve fazer log-in para responder a esta pergunta.

Esta não é a resposta que você está procurando? Pesquise outras perguntas com a tag .