Faça uma coisa por vez
Recentemente tive contato com um trecho de software que me chamou bastante a atenção. A peça era um “resolvedor” de GraphQL que buscava certos dados sobre um Usuário (eu sempre nomeio entidades dos sistemas com os quais trabalho com letras iniciais maiúsculas — acho um bom costume).
Até aí, nada de mais. Entretanto, entretecidas no meio do código, que era bem trivial, estava uma meia-dúzia de mensagens de logging. E isso atrapalhava demais a leitura.
Veja um exemplo:
const GetSomeSpecificUserProperty = async ({
parendId, userEmail, userHash, trackId,
}) =ᐳ {
logger.info(`users.GetSomeSpecificUserProperty: Flow Started! trackId: ${trackId}`);
logger.info(`users.GetSomeSpecificUserProperty: Requesting fields on db trackId: ${trackId}`);
const payload = await getUserProperties({ email: userEmail, userHash: userHash });
if (!payload) {
logger.warn(
`users.GetSomeSpecificUserProperty: Error on retrieving user property trackId: ${trackId}`,
);
return GraphQLMessage({
message: 'Related user does not exist',
});
}
if (payload.parentCustomData) {
logger.info(
`users.GetSomeSpecificUserProperty: parentCustomData is defined, parsing data trackId: ${trackId}`,
);
payload.customData = payload.parentCustomData[parentId];
delete payload.parentCustomData;
} logger.info(
`users.GetSomeSpecificUserProperty: User payload retrieved with success trackId: ${trackId}`,
);
logger.info(`users.GetSomeSpecificUserProperty: Flow ended! trackId: ${trackId}`);
return GraphQLMessage({
message: 'User data retrieved',
status: true,
content: {
payload,
},
});
};
Too much going on
O fato de que é possível fazer uma porção de coisas “ao mesmo tempo” numa mesma função absolutamente não significa que é desejável que criemos funções que fazem uma porção de coisas. A função acima é um excelente exemplo de como uma operação extremamente simples pode ser escrita com código complicado e difícil de ler simplesmente porque resolveu-se que tudo poderia ser feito no mesmo lugar.
É verdade que há ali alguns vícios óbvios (como a famigerada salada de frutas), mas acredito que o pior de tudo, acima de qualquer outra coisa, é a “obfuscação” do código real que está sendo escrito, que fica perdido num emaranhado de outras preocupações secundárias.
Melhorando
1- Foco
O objetivo da função é buscar dados específicos de um usuário relacionados a um determinado “parentId”. Obviamente, extirpei muita informação desse caso específico e já aviso que a ideia toda é o mais puro “quick and dirty”, como bem se pode observar. Mas, ainda assim, o objetivo em si é claro o bastante.
Todavia, além da principal, há mais três preocupações sendo tratadas nessa função, que são:
- Logging
- Encapsulamento do resultado em formato GraphQL
- Tratamento de erros
Alguém poderá argumentar que é okay ter tratamento de erros junto ao “caminho feliz” e, sim, em várias situações esse é mesmo o caso. Mas é saudável para nós, desenvolvedores, conseguirmos pelo menos elencar o que está sendo feito, independente de qual seja a decisão tomada quanto a isso.
Essa mesma função, focada somente no objetivo primário, ficaria assim:
const GetSomeSpecificUserProperty = async ({
parendId, userEmail, userHash, trackId,
}) =ᐳ {
const payload = await getUserProperties({ email: userEmail, userHash: userHash });
if (!payload) {
return {
message: 'Related user does not exist',
};
}
if (payload.parentCustomData) {
payload.customData = payload.parentCustomData[parentId];
delete payload.parentCustomData;
} return {
message: 'User data retrieved',
status: true,
content: {
payload,
},
};
};
Ela ainda tem mais tratamento de erro do que eu acho ideal, mas já está muito mais clara.
2- Clareza
Vemos nesse caso um abuso das arrow functions do Javascript. Estas tem casos de uso muito claros e esse aqui certamente não é um dele. Usá-las, aqui, tira muito da legibilidade do código, pois o que eu tenho em mãos, afinal, é uma função, não uma constante e, portanto, o código ficará muito mais claro se eu chamar a função de função.
Ademais, há um excesso de condicionais, especialmente porque a maioria delas poderia ser eliminada fazendo-se uso de comportamentos default do Javascript. Por exemplo: você não precisa checar se uma chave existe num objeto para invocar o delete nela. Se ela não existir, tudo bem, o runtime se comportará exatamente do mesmo jeito.
Com isso em mente, vamos também aproveitar para delegar o tratamento de erros para o chamador usando um throw. Repare, inclusive, que a função é async (coisa que é capaz de passar despercebida, devido ao uso injustificado de arrow), ou seja: o throw equivale à rejeição da Promise que é gerada por debaixo dos panos pelo runtime.
async function GetSomeSpecificUserProperty(
parendId, userEmail, userHash, trackId,
) {
const payload = await getUserProperties({
email: userEmail, userHash: userHash
}) || throw `Related user ᐸ${userHash} / ${userEmail}ᐳ does not exist`; const customData = payload.parentCustomData || {} // We don't want parents seeing each other custom data, so we clean this up now:
delete payload.parentCustomData const parentData = customData[parentId] || {} return {
message: 'User data retrieved',
status: true,
content: {
{...payload, parentData},
},
};
};
Melhoramos consideravelmente a mensagem de erro, agora incluindo qual usuário foi buscado sem sucesso, enquanto deixamos a cargo do chamador decidir como é que vai transcrever isso para a API GraphQL.
A linha com o delete é pura infelicidade, mas, como disse antes, é uma solução "quick and dirty" e, por ora, temos que aceitar isso e, pelo menos, indicar o motivo dessa bizarrice para o pobre do próximo programador.
Repare que acabamos eliminando todos os if do código, fazendo com que a execução da função "caia por gravidade" sem nenhum desvio de fluxo. A operação em si, afinal, dispensa estruturação.
A “indireção” que tivemos que usar para acessar payload.customData[parentId] poderia ser facilmente solucionada se tivessemos acesso a um operador do tipo ? que evitasse problemas com tentativas de acesso a undefined (típicas do JS) -- mas não o temos, então vamos seguir sem isso e fazer bom uso do operador ||, porque não me importa de qual fonte vem o resultado, se do payload mesmo ou de um objeto que construí apenas para fugir do undefined.
3- Delegação
As outras responsabilidades agora são delegadas ao chamador da função. Repare que ela agora retorna objetos (ou uma Exception, caso haja algum erro) ao invés de GraphQLMessage.
O retorno como mensagem GraphQL se dava porque a função era chamada como “resolver”, mas convenhamos: uma coisa é buscar os dados, outra é se preocupar com o formato da resposta.
Por isso criei alguns “helpers” simples que serão úteis não somente para esta mas para todos os outros “workers+resolvers” do restante da base de código.
Logging
/**
* Run a function while logging the most basic execution
* steps (start, error/finish).
*/
async function LoggingHandlingRunner(title, trackId, func) { logger.info(`${title}: msg=Flow Started!; trackId=${trackId}`) try {
return await func()
}
catch (err) {
if (typeof(err) === 'object') {
level = err.level || 'warn'
logger[level](`${title}: error=${err.systemMessage}; trackId=${trackId}`);
throw err.clientMessage
}
else {
logger.warn(`${title}: error=${err}; trackId=${trackId}`);
throw err
}
}
finally {
logger.info(`${title}: msg=Flow ended!; trackId=${trackId}`)
}}
Como era desejo do desenvolvedor original ter a possibilidade de acompanhar a execução de cada passo via logs, mantive essa possibilidade permitindo que se “envelope” uma função num tratador de logging que trabalha de maneira muito simples:
- Loga início e fim;
- Se a função lançar uma exceção, loga a exceção e a relança.
Repare que decidi relançar a exceção, porque este “decorador” que criei serve apenas para logging. “Faça uma coisa por vez”, afinal.
A ideia é que se passe sempre uma função sem parâmetros. Então, para fazer chamadas com argumentos, o que se faz é criar uma função anônima para “carregar” os argumentos em diante:
return_value = await LoggingHandlingRunner(
logging_title, // Como você quer que o fluxo dessa função apareça nos logs
trackId, // Um identificador de requisição, para propósitos justamente de tracking
() =ᐳ my_function(arg1, arg2, arg3, arg4) // ←----- A função que queremos realmente executar
)Os mais atentos terão reparado que eu eliminei duas mensagens de log da versão original. Pois é. Mensagens de log em sequencia imediata são naturalmente supérfluas, certo? Se toda vez que surgir um logger.info("Alfa") necessariamente seguir-se um logger.info("Beta"), então simplesmente não há sentido em manter ambas as mensagens, certo?
(E se, no seu dia-a-dia, você faz isso para quebrar uma mensagem longa em duas, é provável que esteja errando por usar logging quando deveria usar uma ferramenta de debugging ou profiling.)
GraphQL
/**
* Turns a function that normally returns objects
* into an GraphQL resolver with proper logging
*/
function MakeResolver(func) {
const function_name = func.name return async (args) =ᐳ {
try {
data = await LoggingHandlingRunner(
`${function_name}`, args.trackId, () =ᐳ func(args)
)
}
catch (err) {
if (err === undefined) {
err = `${func.name} has thrown \`undefined\``
}
return GraphQLMessage({ message: err.toString() })
} data.status = data.status || true
data.message = data.message || `${function_name} did not returned a message`
return GraphQLMessage(data)
}
}
Aqui, sim, fazemos tratamento de erros. Minha ideia é a seguinte: facilitar ao máximo a implementação de resolvedores. E, para isso, estabeleço uma regra: se acontecer algum erro, indica-se subindo exceções (com throw). Se tudo der certo, que simplesmente retorne-se um dicionário com message (a galera aqui não leva muito a sério a máxima "no news = good news") e content, que é tudo o que precisa-se para formar uma mensagem de retorno GraphQL segundo os padrões do projeto.
Um resolvedor de exemplo
Agora que temos esses helpers funcionando, criar um resolvedor torna-se extremamente simples.
function DoNothingExceptOnFirstUser(userId) {
if (userId == 1) {
thrown "DoNothingExceptOnFirstUser won't do nothing for User with id=1... oh, wait!"
} return {
message: "Nothing was done"
}
}
4- Versão final
No fim das contas percebi que, ao lançar uma string, esta seria usada tanto para o log interno quanto como resposta na API. E essas tarefas são similares mas diferentes, já que há determinados detalhes de um erro que não queremos que seja do conhecimento do cliente da nossa API.
Por isso implementei, como você já deve ter visto, a possibilidade de se lançar, como exceção, um objeto contendo clientMessage, que é a mensagem que vai para o cliente da API, e systemMessage, que é a mensagem a ser jogada nos logs.
Ademais, como agora fica claro que, se o resolvedor não lançar uma exceção, posso considerar que a requisição foi satisfeita com sucesso, não é mais necessário ficar retornando status: true a toda hora.
async function GetSomeSpecificUserProperty(
parendId, userEmail, userHash, trackId,
) {
const payload = await getUserProperties({
email: userEmail, userHash: userHash
}) || throw {
clientMessage: "User does not exist",
systemMessage: `Related user ᐸ${userHash} / ${userEmail}ᐳ does not exist`
} const customData = payload.parentCustomData || {} // We don't want parents seeing each other custom data, so we clean this up now:
delete payload.parentCustomData const parentData = customData[parentId] || {} return {
message: 'User data retrieved',
content: {
{...payload, parentData},
},
};
};
Vantagens da nova versão
Haverá um artigo nesta série tratando justamente sobre escrever código à prova de idiotas, como eu. A ideia é que deve-se programar pensando que o programador, como eu, é meio besta e facilmente irá esquecer alguma coisa importante, por exemplo.
Antes o desenvolvedor que quisesse criar um novo resolvedor precisava lidar com várias coisas, mas agora só precisa focar em executar a operação pedida e responder num formato extremamente simples. E agora que os erros vêm em forma de exceções, ele também não precisa mais ficar tratando de cada uma que possa surgir e envelopando resultados em GraphQLMessage, mais.
Na hora do code review, temos muito menos código a revisar para cada novo resolvedor implementado. E os testes ficam muito mais simples, já que não precisamos mais testar o fluxo inteiro de requisição e de geração de uma mensage GraphQL. O gerador de mensagens agora tem seus próprios testes, assim como o LoggingHandlingRunner (testes que me indicaram, inclusive, que o último info precisava ficar dentro de um bloco de finally).
Ademais, numa eventual adaptação desse código, não temos mais o formato final (GraphQL) todo enroscado no meio dos resolvedores. Com alterações muito pontuais podemos aproveitar todos os resolvedores e prover uma API REST, por exemplo (sim, é um exemplo bizarro, mas a vida está cheia de coisas bizarras, ambos sabemos bem disso).
“Uma coisa só nem sempre é possível”
Talvez, mas eu poderia aposta que o é em 99% dos casos do dia-a-dia.
Inclusive, se você está pensando em alguma função que precisa fazer a composição do resultado de diversas outras, eis aqui a resposta: esta função deve fazer uma coisa só. No caso: composição. Se dentro dela você implementa mais lógica do que o necessário, já está fazendo mais que uma coisa.
Futuro
O tratador de exceções que criei é propenso a erros por parte do desenvolvedor, que pode esquecer de passar uma ou ambas as chaves do objeto. O ideal seria haver uma instanciação mais formal com new. Mas isso não é pra agora. Estou fazendo isso no meu tempo livre, afinal...
Resumo
- Faça uma coisa por vez.
Curtiu? Então assine a minha newsletter e receba conteúdo interessante em pequenos drops direto na sua caixa de e-mails:
Sugestão de leitura: Team Topologies: Organizing Business and Technology Teams for Fast Flow
