Dica para melhorar classe

Eu removeria a flag de erro criando um array de erros. Cada erro seria adicdionado à esse array e, através de um método hasErrors() eu faria o mesmo que essa flag hoje faz sem sujar o código ou violar o encapsulamento:

class Correios {

 private $errors = array();

 // ...

 public function hasErrors() {
 return ( count( $this -> errors ) != 0 );
 }
}

Definiria a URL do WebService dos Correios numa constante de classe, assim, se eventualmente esse valor mudar, você teria fácil acesso à sua alteração, já que normalmente elas são definidas no começo da classe.

Sem contar o fato de que se você vier a precisar dessa mesma URL em outras partes do código, quando e se precisar alterar, não precisará fazê-lo em todo o código:

class Correios {

 const WSURL = 'http://websro.correios.com.br/sro_bin/sroii_xml.eventos';

 // ...

 public function search() {

 $cURL = curl_init( self::WSURL );

 // ...
 }
}

Alteraria todas as propriedades atualmente públicas para particulares (porque privada é feio :P) e criaria getters para todas as informações passíveis de serem resgatadas:

class Correios {

 private $result;

 // ...

 public function getResult() {
 return $this -> result;
 }
}

Removeria todas as propriedades desnecessárias à classe. Se você tem uma propriedade que vai armazenar o resultado, porque não armazenar nessa mesma propriedade o hash e a Data Final também?

Na hora de utilizar, já que vai ocorrer um loop, você simplesmente não lista tais informações.

Por preferência pessoal, faria da propriedade de resultado um objeto stdClass, fica mais elegante. Porém, como a mudança ocorrida no PHP 5.4 isso pode gerar uma micro, porém presente, perda de desempenho.

Por fim, totalmente opcional, seria abstrair a forma como o método de busca faz a requisição. Imagina que o servidor que usar esse código não tenha a cURL instalada? seu script não roda:

interface Adapter {

 public function getData( $url );
}

class cURL implements Adapter {

 public function getData( $url ) {

 $cURL = curl_init( $url );

 // Set Options

 $data = curl_exec( $cURL );

 curl_close( $cURL );

 return $data;
 }
}

class Socket implements Adapter {

 public function getData( $url ) {

 /**
 * Faz a mesma coisa, porém utilizar fopen() / fread() / fclose()
 *
 * Ou file_get_contents() se estiver com preguiça :P
 */
 }
}

class Correios {

 public function __construct( Adapter $adapter, $object ) {

 // Faz a verificação e...

 $this -> search( $adapter, $objeto );
 }

 public function search( Adapter $adapter, $objeto ) {

 $data = $adapter -> getData( self::WSURL );

 // ...
 }
}

$correios = new Correios( new cURL );

// ou

$correios = new Correios( new Socket );

if( ! $correios -> hasErrors() ) {

} else {

}

Pode ser que tenha alguma coisa incorreta, mas eu escrevi agora e sem um servidor para testar (meu notebook quebrou de novo).

Veja que essa última dica está bem incompleta, afinal, você precisa informar opções muito específicas à este serviço para a CURL, logo, tais implementações devem fornecer um meio de fazê-lo sem que você precise escrever as opções dentro das classes CURL/Socket.

Bons estudos :thumbsup:

Muito obrigado Bruno Augusto, sempre me da altas dicas! vou reescrever a classe da forma que você disse./applications/core/interface/imageproxy/imageproxy.php?img=http://forum.imasters.com.br/public/style_emoticons/default/clap.gif&key=ab7a79d2320a1ded436b2ab0fea47e116ade502c5a2c7167044566e6dce34a83" alt="clap.gif" />

Muito bom Bruno Augusto, me ajudou muito também a entender alguns conceitos :)

Só uma dúvida...

O Adapter na classe correio seria legal ficar armazenado em uma propriedade?

Vamos supor que haja outro método que faça uso desse Adapter, com isso não preciso passa-lo novamente...

Desde já agradeços!

O Adapter na classe correio seria legal ficar armazenado em uma propriedade?

Eu até cheguei a escrever dessa forma, mas depois eu vi que ele invoca o método que faz a busca dentro do próprio construtor e dada a simplicidade da classe, se torna desnecessária essa propriedade.

Outra coisa que faltou mencionar seria alterar a visibilidade do método da busca para private, afinal o próprio construtor o chama.

Mas se houverem outros métodos, com outras funcionalidades, a consideração do adaptador na propriedade deve ser levada em conta sim, assim como a da visibilidade citada acima, ignorada, afinal nem todas as tarefas serão disparadas todas as tarefas de uma só vez.

E claro, nessa última hipótese, o método que faz a busca não deveria ser invocado no construtor, pois estaria poderia estar fazendo algo que o usuário não queira, pesando desnecessariamente.

Fiz as alterações propostas porém como sou iniciante não sei sei fazer os arrays de respostas serem apenas arrays. sempre são SimpleXMLElement Object confira

Adapter.php

<?php

interface Adapter {

 public function getData($url, $objeto);
}

cURL.php

<?php

class cURL implements Adapter {

 public function getData($url, $objeto) {
 $cURL = curl_init($url);
 curl_setopt($cURL, CURLOPT_RETURNTRANSFER, TRUE);
 $dados = array('Usuario' => 'ECT', 'Senha' => 'SRO', 'Tipo' => 'L', 'Resultado' => 'T', 'Objetos' => $objeto);
 curl_setopt($cURL, CURLOPT_POST, TRUE);
 curl_setopt($cURL, CURLOPT_POSTFIELDS, $dados);
 $xml = curl_exec($cURL);
 curl_close($cURL);

 return $xml;
 }

}

 

 

 

<?php

class Correios {

 private $errors = array();
 public $resultado = array();

 const WSURL = 'http://websro.correios.com.br/sro_bin/sroii_xml.eventos';

 public function __construct(Adapter $adapter, $objeto) {

 if (preg_match('/^[a-zA-Z]{2}[0-9]{9}[a-zA-Z]{2}$/', $objeto)) {
 $this->search($adapter, $objeto);
 } else {

 $this->errors[] = 'Código inválido';
 }
 }

 public function search(Adapter $adapter, $objeto) {

 $data = $adapter->getData(self::WSURL, $objeto);
 $this->resultado['hash'] = md5($data);

 $xml = simplexml_load_string($data);

 if (!$xml->error) {
 $this->resultado['status'] = $xml->objeto->evento->descricao;

 foreach ($xml->objeto->evento as $evento) {
 $temp['DATA'] = $evento->data . ' ' . $evento->hora;
 $temp['LOCAL'] = $evento->local . ' ' . $evento->cidade . ' ' . $evento->uf;
 $temp['ACAO'] = $evento->descricao . '';
 if ($evento->destino) {
 $temp['DETALHES'] = 'Em trânsito para ' . $evento->destino->local . ' - ' . $evento->destino->cidade . '/' . $evento->destino->uf;
 } else {
 $temp['DETALHES'] = '-';
 }
 $this->resultado['rastreio'][] = $temp;

 $this->resultado['dataInicial'] = $evento->data;
 }
 $this->resultado['rastreio'] = array_reverse($this->resultado['rastreio']);
 } else {

 $this->errors[] = 'Código não encontrado';
 }

 return $this->resultado;
 }

 public function hasErrors() {
 return ( count($this->errors) != 0 );
 }

}

Quando tenso acessar os dados assim, gera um erro:

<?php
include 'Adapter.php';
include 'Correios.php';
include 'cURL.php';

$correios = new Correios( new cURL, 'CJ160712482US' );

echo $correios['hash'];

Alguém pode explicar o motivo?

Você diz que gera um erro... Mas qual erro?

E dessa forma que você fez você limita a interface forçando a variável $objeto estar presente.

Da forma que eu passei, mesmo que simplória e carente de recursos, aquele Adapter pode ser usado para qualquer requisição externa que você queira fazer, pois ele requer apenas o URL alvo.

O erro foi por causa da minha ignorância, quando tento acessar um valor do Correios tentei pegar como array simples por exemplo

$correios = new Correios( new cURL, 'CJ160712482US' );

echo $correios['hash'];

o correto seria

echo $correios->resultado['hash'];

--

O melhor seria tirar o objeto de la então e chamar ele só dentro Correios? e outra aquele loop que eu faço para organizar o xml dentro do array não é muito bagunçado não?