@kirill-93

Правильно ли так организовывать код?

Здравствуйте. Понадобилось разработать АПИ для получения данных из другого АПИ.
Сама по себе задача элементарная, но меня мучает вопрос, как правильнее это сделать.
Вот как сделал я:

1) Создал директорию Services в app и добавил в автозагрузку в композере. Создал класс Partner, который умеет отправлять и получать песни.

//App\Services
class Partner
{
    /**
     * Returns an object of song of certain singer.
     *
     * @param  string  $singer
     * @param  string  $song_name
     * @return array
     */
    public function getSong(string $singer, string $song_name) : array
    {
        try {
            $json = file_get_contents('partner-domain.com/api?singer=' . $singer);
        } catch (\Exception $e) {
             //throw new Exception or return object with status?
        }

        $songs = json_decode($json, true);
    
        //Filter songs by given name
        $result = array_filter($songs, function($song) use ($song_name) {
            return mb_strtolower($song['name'] === mb_strtolower($song_name));
        });

         //We need to return only 1 song
        return $result[0] ?? [];
    }

    /**
     * Send a song to API.
     *
     * @param  string  $singer
     * @param  object  $song
     * @return void
     */
    public function sendSong(string $singer, object $song) : void
    {
        try {
            $curl = curl_init('partner-domain.com/api/send');
            /*curl body, postfields etc...*/
            curl_exec($curl);
            curl_close($curl);
        } catch (\Exception $e) {
            //throw new Exception or return object with status?
        }
    }
}


2) Создал роут и метод в контроллере:
//App\Http\Controllers

class ApiController()
{
    private $partnerService;

    public function __constructor()
    {
        $this->partnerService = new App\Services\Partner();
    }

    /**
     * Returns an object of song of certain singer.
     *
     * @param  string  $singer
     * @param  string  $song_name
     * @return string
     */
    public function getSong(string $singer, string $song_name) : string
    {
         $song = $this->partnerService->getSong($singer, $song_name);

         return response()->json($song);
    }

    /**
     * Send a song to API.
     *
     * @param  string  $singer
     * @param  object $song
     * @return string
     */
    public function sendSong(string $singer, object $song) : void
    {
         $this->partnerService->sendSong($singer, $song);
    }
}


У меня несколько вопросов по этому коду:
1) Как правильно обрабатывать ошибки с доступом к партнерскому сервису? В классе Partner в методе getSong правильнее выбрасывать исключение и отлавливать его в контроллере (но для этого придется все вызовы методов класса Partner оборачивать в try-catch) или просто возвращать объект с ключом 'status', куда писать 'ok' в случае успеха и 'error' в случае, если что-то пошло не так, примерно вот так:
//App\Services\Partner
public function getSong(string $singer, string $song_name) : array
    {
        try {
            $json = file_get_contents('partner-domain.com/api?singer=' . $singer);
        } catch (\Exception $e) {
            return [
                'status' => 'error',
                'message' => $e->getMessage();
            ];
        }

        $songs = json_decode($json, true);

        //Filter songs by given name
        $result = array_filter($songs, function($song) use ($song_name) {
            return mb_strtolower($song['name'] === mb_strtolower($song_name));
        });

        //We need to return only 1 song
        if (isset($result[0])) {
             return [
                'status' => 'ok',
                'data' => $result[0];
            ];
        } else {
             return [
                'status' => 'error',
                'message' => 'not found';
            ];
        }
}


2) Правильно ли так подключать свои классы в контроллере (через конструктор)?
3) Вопрос аналогичен первому, но касается метода sendSong. Что если там произойдет ошибка? Как правильнее возвращать ее из класса в контроллер и из контроллера пользователю?
4) В php ведь нельзя указывать два типа возвращаемого значения? И нельзя указывать mixed. Это значит, что если в методе getSong такой песни не найдется, то я не смогу вернуть, например, null вместо массива? Придется возвращать пустой массив. Это правильно?

Буду очень благодарен за развернутые ответы.
Спасибо.
  • Вопрос задан
  • 360 просмотров
Решения вопроса 3
myks92
@myks92 Куратор тега PHP
Нашёл решение — пометь вопрос ответом!
  1. Лучше try/catch. Если использовать так, то ваша форма ответа может быть разная в зависимости от сервиса. Сервис выбрасывает ошибку, а в контроллере или промежуточном слое она ловится и форматируется. При этом контроллеры могут быть разные и у каждого свой формат ответа: Web, API, Console
  2. Лучше передавать в сам конструктор и использовать DI.
  3. В методе просто выбрасываете исключение throw new Exeption('Ошибка!'); А в контроллере try/catch.
  4. Пока что нет. Будет только в php8. Метод get должен возвращать ошибку или данные. Метод find должен возвращать null или данные.

Ответ написан
glaphire
@glaphire Куратор тега PHP
PHP developer
1) советую использовать обертку над курлом, нп. Guzzle
2) правильно, но это стоит делать через autoinjection (di container), а не new, почитайте ларовскую доку
3, 4) возвращать данные в случае успеха, исключение в случае ошибок, так не будет проблем с двумя типами возвращаемых значений. Исключения можно уже хендлить в месте вызова функции getSong, т.е. try catch в контроллере и в catch подготавливать респонс в случае ошибки
Ответ написан
Комментировать
JhaoDa
@JhaoDa
LaravelRUS Team
1, 2, 3) В документации ларавел, которую ты до сих пор не прочитал, написано, как правильно.

4) Можно вернуть null вместо массива. Кто запрещает?

Ещё стоит закопать стюардессу, перестав юзать curl и начать юзать guzzle/guzzle.
Ответ написан
Пригласить эксперта
Ответы на вопрос 1
dmitriylanets
@dmitriylanets
веб-разработчик
1) из класса Partner должно идти как минимум одно исключение PartnerException
все исключения в классе Partner будут "переварены в одно" PartnerException
в вашем случае вы можете добавить еще наследника и PartnerNotFoundException
логика становиться более лаконичной:
//App\Services\Partner
public function getSong(string $singer, string $song_name) : array
    {
        try {
            $json = file_get_contents('partner-domain.com/api?singer=' . $singer);
        } catch (\Exception $e) {
            throw new PartnerException($e->getMessage());
        }

        $songs = json_decode($json, true);

        //Filter songs by given name
        $result = array_filter($songs, function($song) use ($song_name) {
            return mb_strtolower($song['name'] === mb_strtolower($song_name));
        });

        //We need to return only 1 song
        if (!isset($result[0])) {
              throw new PartnerNotFoundException(sprintf(
                   'parnter not found by %s and %s', $singer, $song_name
            ));
        } 

       return [
                'status' => 'ok',
                'data' => $result[0];
      ];
}


- в контроллере try catch если нужно поймать
public function getSong(string $singer, string $song_name) : string
    {
         try{
             $song = $this->partnerService->getSong($singer, $song_name);
         }
        catch(PartnerNotFoundException $ex){

            //п.с не помню как в лаевел но смысл поняли
             return response()->status(404)->send();
        }

         return response()->json($song);
    }

2) через конструктор
3) аналогично п1
4) метод называется getSong получить песню, это значит все альтернативные варианты загоняем в исключения, нет песни - исключение, ошибка сервиса, исключение
Ответ написан
Комментировать
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Похожие вопросы