Gambiarra ou não?

13 respostas
ahlx

Este meu método saca de uma conta bancária se o valor a sacar for menor que o saldo da conta e se o valor a sacar for maior que 5 reais. Se tiver essas situações ok, deverei efetuar o cálculo e retornar o saldo, comecei com o método retornando float. Mas também quero que se não der certo, retorne uma mensagem dizendo o porque, só que aí, como o método era float, não dava, então, mudei pra string e converti o saldo para string. O problema é que pra mim isso é gambiarra e não tá correto. O que vocês fariam? Ou pode ser assim mesmo?

public String sacar(float valorSacar) { String saldoResultado; // somente saca se existir saldo bancário if (valorSacar <= saldo) { if (valorSacar > 5) { saldo = saldo - valorSacar; saldoResultado = Float.toString(saldo); return saldoResultado; } else { return "Valor a sacar é menor que R$ 5,00."; } } else { return "Valor a sacar é maior que saldo disponível."; } }

13 Respostas

doug

Olá ahlx
Entaum gambiarras também funcionam… mas deve ser usada com muita caltela…
A respeito do sem código… eu usaria os recursos de excetions… fazendo o método
que chamou esse método de sua classe fizesse try catch… obrigando a fazer o tratamento.
Aacho que assim fica mais elegante

flwss…
espero ter ajudado

felipealbuquerque

O que você acha de lançar alguma checked exception caso a operação não tenha sido efetuada com sucesso? Você pode criar algumas exceptions bem auto-descritivas, tipo um SaldoInsuficienteException.
É mais seguro do que um tratamento com Strings.

ahlx

Entendi, obrigado. Quer dizer que eu posso voltar meu método para float normalmente e caso dê errado, será lançado exceções. Valeu pessoal.

Tenho outra dúvida se não for muito encômodo. É sobre get e set. Neste caso, meu método parece que faz os dois ao mesmo tempo, ele seta minha variável saldo e ainda retorna o novo saldo. É correto?

Obrigado!

jcvijr

Não tem problema usar gets ou sets, só tenha cuidado com os problemas que seu método possa apresentar caso o acesso seja concorrente. O saldo é uma variável que pertence à região crítica, e você deve garantir que apenas um processo acessa essa variável por vez.
Não sei se apenas colocando seu método como synchronized seria uma boa solução…

Sugestões?

Linkel

Cara, na minha opnião não há nenhuma gambiarra em seu método!
Está estritamente lógico e definido!
Como o amigo jcvijr disse, só tenha cuidado na hora de executá-lo, garantindo que seja executado do início ao fim sem concorrência.

Um abraço!

sergiotaborda

E vc toda a razão. É gambiarra sim!

Como saber se é gambiarra ? Simples: Se o conteudo logico da variável pode ser mais do que 1 é gambiarra.
No caso, o ocntudo pode ser um numero que representa o saldo ou uma mensagem que não representa o saldo. É gambiarra.

O mecanismo para evitar isto é usar exceptions.

Não vi nenhum uso de get/set no seu codigo. Vc está utilizando as variáveis internas (suponho). A única coisa a ter cuidado é que não deve ser possivel fazer um get enquanto o metodo está correndo. Para isso precisa de sincronizar o get com o sacar.

Nota: na realidade o metodo sacar não poderia estar na conta porque tem um conflito direto com o método setSaldo(). A conta deveria ter um campo package e um get publico para ele e nenhum set. E um objeto Transferencia , do mesmo pacote deveria executar as operações apenas alterando o saldo se isso fosse possivel.

ahlx

Agradeço pelas dicas recebidas até agora.

Realmente Sérgio, eu não criei os métodos get e set, não sei se isso é certo, mas eu pensei assim, só os crio quando eu precisar, e realmente quando fui chamar esta classe por outra, tive esssa necessidade e criei. Mas, tenho uma dúvida: Preciso criar métodos set para as mesmas variáveis que usei no construtor de novo?

peczenyj

Vc tem 2 escolhas:

A primeira é fazer o método retornar um inteiro que, se for 0 significa que foi tudo bem, -1 se ocorreu um erro, etc.

A segunda, MUITO mais elegante, seria vc criar uma exception especial para esse caso e informar na assinatura do método que ele pode lançar uma exception desse tipo. Quem for chamar o método de sacar dinheiro tem que tratar essa exception e assim a coisa fica bonito.

Agora, eu vejo 2 problemas nesse modelo:

  • Usar float para dinheiro traz alguns tipos de problema. Com float vc pode ter muitos erros de arredondamento. IMHO vc poderia optar por usar double (ou BigDecimal ?). Uma outra opção seria usar um inteiro – o número de centavos .
  • Vc esta atrelando regras demais nesse método. Vc deveria poder sacar qualquer quantidade de dinheiro possivel. Acho que vc poderia colocar essas regras em outro componente. Imagine que vc tem casos assim: cliente especial pode sacar qq coisa, cliente normal só pode sacar no mínimo X, cliente VIP tem alguma vantagem, no natal qq um pode sacar o que bem entender… seria o caso de vc pensar a respeito.

Por fim, pense em testes unitários.

bzanchet

O seu código é uma baita duma gambiarra. Se é pra retornar um float, não use String pra isso.

E eu também discordo do uso de exceções. Eu prefiro o uso de exceções apenas para situações inesperadas - e saldo insuficiente não é exatamente uma situação inesperada. É algo que deve acontecer durante o uso “normal” do programa, mesmo.

Proponho que indicar o saldo resultante não seja responsabilidade deste método. E, além disso, se achas, mesmo, que este método deve ser responsável por sacar e ainda por definir indicar motivo do erro (caso falhe), diria para retornar um enum especificando o resultado da ação (funcionaria como o int que o peczenyj sugeriu, só que typesafe e - mais importante - comunicaria melhor a intenção do retorno do método, algo que não fica claro quando se retorna apenas um inteiro).

peczenyj

Vc prefere isso?

bool resultado = MeuObjeto.SacarDinheiro(500); if (resultado){ mostraMensagem("LEGAL! VC SACOU DINHEIRO!"); } else { mostraMensagem("DROGA! VC NAO SACOU DINHEIRO!"); }

Ou isso?

Resultado resultado = MeuObjeto.SacarDinheiro(500); if (resultado.equals(Resultado.SUCESSO)){ mostraMensagem("LEGAL! VC SACOU DINHEIRO!"); } else { mostraMensagem("DROGA! VC NAO SACOU DINHEIRO! Motivo " + resultado.getMotivo()); }

Ou isso ?

try{ MeuObjeto.SacarDinheiro(500); mostraMensagem("LEGAL! VC SACOU DINHEIRO!"); } catch(Exception e){ mostraMensagem("DROGA! VC NAO SACOU DINHEIRO! " + e.getMessage()); }

Ou quem sabe isso ?

try{ MeuObjeto.SacarDinheiro(500); mostraMensagem("LEGAL! VC SACOU DINHEIRO!"); } catch(NaoTemDinheiroNaContaException e){ mostraMensagem("DROGA! VC NAO TEM DINHEIRO!" ); } catch(SaqueAbaixoDoLimiteException e){ mostraMensagem("DROGA! VC TEM DINHEIRO MAS..."); } catch(ProblemaBizarroException e) mostraMensagem("CACILDA!!! Deu um problema mó bizarro, saca so: " + e.getMessage()); }

IMHO tudo é uma questão de bom senso :slight_smile:

bzanchet

peczenyj:
Vc prefere isso?
IMHO tudo é uma questão de bom senso :)

Exato, é tudo uma questão de bom senso - por isso eu digo que “prefiro” assim, e não que “deve ser”. =)

Estava procurando o texto do DHH em que ele explica o porque do “save()” do ActiveRecord retornar “false” ao invés de lançar uma exceção, em caso de falha - mas meu “Ágile Web Development…” está emprestado (e não achei no google). Certamente que a explicação dele é melhor do que a minha, mas esta é a que resta, por enquanto.

Eu acredito na filosofia de usar exceções para condições de erro, inesperadas, e não para condições esperadas. Estas deveriam ser tratadas com lógica condicional, comum. O resultado final é um código mais legível e com um fluxo mais fácil de ser entendido (pode ser mais fácil de ler num primeiro momento, mas quando a complexidade aumenta um pouco o uso de exceções já tornam a lógica mais difícil de ser seguida).

Qual dos dois tem a lógica mais fácil de seguir? E qual é mais fácil de modificar? (e se notificarBancoCentral() e cobrarCPMF() ainda lançassem 3 exceções cada um, sendo uma delas um erro irrecuperável - MorreuOBancoDeDadosException - e duas apenas notificações de condições esperadas?)

class CaixaEletronico { public Resultado sacarComCartao(float valor) throws SQLException { Resultado resultado = MeuObjeto.SacarDinheiro(500); if (resultado.equals(Resultado.SUCESSO)){ notificarBancoCentral(); cobrarCPMF(); //esse ta meio desatualizado =) } return resultado; } } ou

class CaixaEletronico {
  public void sacarComCartao(float valor) throws SQLException, NaoTemDinheiroNaContaException, SaqueAbaixoDoLimiteException,ProblemaBizarroException  {
    MeuObjeto.SacarDinheiro(500);
    notificarBancoCentral();
    cobrarCPMF(); //esse ta meio desatualizado =)
  }
}

Eu prefiro poupar as exceções para tratamento de erros legítimos.

peczenyj

Acho que o uso de exception ou não fica mais claro em exemplos mais complexos.

Eu posso querer delegar a quem chama o método sacarDinheiroComCartaoVipPlus o tratamento de diversos problemas OU engolir tudo e dizer “não deu”, colocando em um log o motivo (se bem q nada me impede de fazer algo misto). Se eu pensar em uma transação grandinha, posso querer lançar uma exception e interromper o fluxo de execução, disparando um rollback() no catch, poupando diversos ifs encadeados.

bool status = doIt(X); if(status){ status =doIt(Y); if(status){ status =doIt(Z); if(status) fimLegal(); else fimNaoLegal(); // ok, esse metodo pode estar em outro(s) lugar(es) } } }

try{ doIt(X); doIt(Y); doIt(Z); fimLegal(); } catch(DoItException die){ fimNaoLegal(); }

Felizmente temos muitas opções :slight_smile:

dqr_2008

Ah carah… se não arrumar outro jeito, entenda como ADAPTAÇÃO TECNICA não como gambiarra!!!

flwww

<–Vo muda lah pro Polo Norte, pq lah – TUDO É FREE -->

Criado 24 de fevereiro de 2008
Ultima resposta 25 de fev. de 2008
Respostas 13
Participantes 9