Give me feedback

A Laravel 5.0 Model that represents a city found directly from the html page of another website

This class has 2 important methods. "findByNomeEstado()" search for an string at the database and "findFromIbge()" can search for that same string, but this time, we use "phpQuery" library to inspect the html from another webpage and find the elements we want, in order to fill some attributes and save at the database.

Cidade.php
<?php namespace App; use Illuminate\Database\Eloquent\Model; use Guzzle\Http\Client; use Guzzle\Http\Exception\ClientErrorResponseException; use Illuminate\Database\Eloquent\ModelNotFoundException; use Carbon\Carbon; class Cidade extends Model { /** * @var string URL de conexão com o site do IBGE. */ protected static $url = 'http://www.ibge.gov.br/home/geociencias/areaterritorial/area.php'; /** * @var int Quantidade de meses limite para consultar a cidade na base de dados. */ protected static $months = 12; protected $hidden = ['id', 'created_at', 'updated_at']; protected $fillable = ['codigo_ibge']; public function scopeEstado($query, $estado) { if (!empty($estado)) { return $query->whereEstado($estado); } return $query; } /** * Procura um model usando os campos nome e estado. * @param string $nome * @param string $estado * @throws ModelNotFoundException * @return Cidade */ public static function findByNomeEstado($nome, $estado = null) { return self::where('nome', '=', $nome) ->whereEstado($estado) ->where('updated_at', '>', Carbon::now()->subMonths(self::$months)) ->firstOrFail(); } /** * Procura um model a partir do site do IBGE. * @param string $nome * @param string $estado * @throws ModelNotFoundException * @return Cidade */ public static function findFromIbge($nome, $estado = null) { try { $client = new Client(); $request = $client->get(self::$url, array(), array()); $query = $request->getQuery(); $query->add('nome', $nome); $response = $request->send(); $html = $response->getBody(true); $html = \phpQuery::newDocumentHTML($html, 'utf-8'); // construímos o array com as cidades encontradas $array_cidades = array(); foreach ($html->find("div#miolo_interno > table > tr") as $c => $tr) { // retiramos a primeira linha, pois este é o cabeçalho da tabela if ($c == 0) { continue; } $dados = array(); // 0=codigo_estado, 1=estado, 2=codigo_ibge, 3=nome, 4=area_km2 foreach (pq($tr)->find("td") as $key => $td) { $col = pq($td); $dados[$key] = $col->html(); } // filtramos o estado antes de preencher o array de cidades encontradas if (!empty($estado) && ($dados[0] != $estado && strcasecmp($dados[1], $estado) != 0)) { continue; } $array_cidades[] = $dados; } // validamos a quantidade de cidades encontradas if (count($array_cidades) == 0) { throw new ModelNotFoundException(); } else if (count($array_cidades) > 1) { throw new \LengthException(); } // setamos os parâmetros do model $cidade = self::firstOrNew(['codigo_ibge' => $array_cidades[0][2]]); $cidade->setAttribute('nome', $array_cidades[0][3]); $cidade->setAttribute('estado', $array_cidades[0][1]); $cidade->touch(); return $cidade; } catch (ClientErrorResponseException $e) { abort(404, "O endereço do Correios informado não esta acessível."); } } }

Reactions 2 reactions

seb7 SebSept commented on this code
-1
Hello,

This class is an ORM model, it handle domain logic and persistence logic.
You should not handle the scrapping here. That's different concern, task.
This way it will be easier to test. You should use a separed class to handle this.

Another problem I see is that every time the function findFromIbge() is called, the distant webpage is scrapped. That's a long operation. I suggest to store fetched results in a database for the script to be faster for the next calls.
seb7 SebSept commented on this code
1
Hello,

This class is an ORM model, it handle domain logic and persistence logic.
You should not handle the scrapping here. That's different concern, task.
This way it will be easier to test. You should use a separed class to handle this.

Another problem I see is that every time the function findFromIbge() is called, the distant webpage is scrapped. That's a long operation. I suggest to store fetched results in a database for the script to be faster for the next calls.

Comment on this code:

Please Login or Register to leave a comment.