[SP-pm] Printer.pm - aberto a comentários.

[Solli Honorio]

Oi Thiago, antes de qualquer coisa preciso declarar os meus parabéns para a
tua iniciativa e coragem de colocar o teu código a comentários do pessoal. O
meu comentário não será de precisão pois estou sem muito tempo para isto,
mas acho que já vão ajudar.

1o. Enfatizando o Lorn, considere imediatamente utilizar o Module::Starter (
http://search.cpan.org/~petdance/Module-Starter-1.54/lib/Module/Starter.pm),
utilizar a estrutura de diretórios e o padrão de documentação POD;

2o. Como disse o Luis, utilize o Perl::Tidy;

3o. considere trocar o 'our %alarmCode' por 'my %alarmCode'. A diferença
básica está na definição de escopo desta variável;

4o. Recomendo utilizar fortemente uma variável comum ao invéz de um file
handler, tecnicamente chamados isto de 'anonymous filehandle'. Primeiro é
preciso entender que file handler é uma variável global, então se vc tiver
um módulo (tal como o teu) que possa ser chamado por mais de uma vez no
mesmo programa, isto significa que você terá uma oportunidade muito grande
de gerar um bug difícil de achar. Não vou entrar nos detalhes aqui, mas vc
pode fazer um teste simples: abre o mesmo arquivo com o mesmo filehandle em
duas posições diferentes do código, e veja o resultado. Outra vantagem no
anonymous filehandle, é que o recurso fica aberto enquanto está no escopo,
então quando vc sai do escopo, o recurso é encerrado automaticamente. Em
situação normal isto significa que vc não precisa do 'close ($file)';

5o. Não sei se é legal algumas verificações que você faz no new (o
checkIsLive, por exemplo). Pois aqui tem 2 problemas para mim. Uma
verificação deste tipo no new só faz sentido se caso o recurso não existir,
você para completamente a execução do programa ( die if ! checkIsLive ). Do
contrário não faz sentido esta verificação que vc faz, pois você precisaria
fazê-lo em outros lugares. Então, a minha recomendação é : só deixe no new
as atribuições que só fazem sentido no new (tipo definição de valores
padrão, caso não tenha sido definido na criação do objeto);

6o. ainda no new, vc está quase que exeutando tudo já no new. isto não faz
muito sentido em OO, pense bem nisto;

7o. o checkIsLive (eu colocaria apenas is_live) tem que estar próximo de que
vai utilizar, neste nestido eu poderia utilizar no snmpSession (por
exemplo);

8o. ainda no snmpSession, eu recomendo não misturar retorno de erro com
retorno de dados. Então eu recomendo :
 * sempre utilizar o return para o erro;
* passe o resultado através de alguma referência na passagem de parâmetros,
ou já coloque na estrutura do teu $self (pois vejo que você utiliza isto no
futuro)

9o. considere a definição do arquivo 'enterprise.txt' ser uma opção mas com
valor padrão 'enterprise.txt';

10o. por uma questão mais pessoal, eu não gosto muito de coisas como
checkIsLive, ou loadAllSNMP. Isto é mais Java like, eu gosto mais separar
por '_', então eu faria is_live, all_snmp. Mas isto é questão pessoal.


Em 28 de janeiro de 2010 22:51, thiago User <thiago em nerdsland.net> escreveu:

> Ok, Ok. Estou aprendendo a usar o git... coloquei uma versão do módulo
> Printer.pm que eu uso no sistema de impressão que criamos na Petro - que
> será batizado de SNMPrinter. Ainda não esta completo, mas estou aberto
> aos comentários e criticas dos colegas mais experientes.
>
>
> git://gist.github.com/289308.git
> http://github.com/thiagoglauco/SNMPrinter
>
>
> _______________________________________________
> SaoPaulo-pm mailing list
> SaoPaulo-pm em pm.org
> http://mail.pm.org/mailman/listinfo/saopaulo-pm




-- 
"o animal satisfeito dorme". - Guimarães Rosa
-------------- Pr?xima Parte ----------
Um anexo em HTML foi limpo...
URL: <http://mail.pm.org/pipermail/saopaulo-pm/attachments/20100130/25ab025f/attachment-0001.html>