Laravel Türkiye Discord Kanalı Forumda kod paylaşılırken dikkat edilmesi gerekenler!Birlikte proje geliştirmek ister misiniz?

Laravel Video Data (youtube/vimeo) API

sseffa

Merhaba,

Laravel'de ilk geliştirdiğim paketi sizinle paylaşmak istiyorum. Bu pakette vimeo ve youtube video/user id'sini kullanarak video bilgilerini ve kanala ait videoların listesini alabilirsiniz.

https://github.com/sseffa/laravel-video-api

Teşekkürler

sergin

@sseffa, Laravel 4 içerik yönetim sisteminden sonra bu yeni çalışmanız için de sizi kutluyorum. Bununla birlikte çalışmanızla ilgili önerilerim olacak. Bu önerileri değerlendirirken Kapağının Altındaki Laravel: Facade (http://serginari.com/blog/kapaginin-altindaki-laravel-facade/) ve Laravel Paketleri Laravel Yolundan Gidiyor mu? (http://serginari.com/blog/laravel-paketleri-laravel-yolundan-gidiyor-mu/) başlıklı yazılarımı okumanızı kuvvetle tavsiye ederim.

Laravel Facade'ın en önemli gerekçesi test edilebilirliği kolaylaştırmak amacıyla, "statik property ve metodlar barındırmayan sınıfları" statik sözdizimi aracılığıyla kullanabilmektedir.
1- Paketinizde Facade kullanmanıza karşın VideoApi sınıfınız statiktir. Sınıfınız oluşturucu metodla oluşturulmalı ama Facade aracılığıyla static gibi erişilebilmelidir.
2- Örneklerinizde sınıfı $videoApi = VideoApi::getInstance('youtube'); şeklinde açıktan oluşturmak zorunda kalıyor, ayarlamalar v.s. yapıp birkaç metod kullandıktan sonra çıktı alıyorsunuz. Laravel'de beklediğimiz ise, şöyle bir sözdizimidir: VideoApi::setType('youtube')->setId($id)->getVideoList();
hatta sınıf metodları uygun ayarlanarak kısaca,
VideoApi::getVideoList('youtube', $id)
diyebilmeliyiz. Böylece kod tekrarından da kurtulmuş oluruz. VimeoApi ve YoutubeApi'nin birçok metodunun gövde kısımları birbirinin tıpatıp aynısı olduğuna göre bence doğrudan VideoApi sınıfında protected bazı özellikler ayarlanarak vimeo veya youtube tipleri ve id ayarlanabilir. Yine bazı protected metodlarla da kod tekrarlarında uzak durulabilir. Böylece rota tanımlamalarınız da
Route::get('video/{type}/{id}/{listveyadetay}', function (......... gibi olabilir.
3- Paket yapımında sık karşılaştığım ve sizin pakette de gördüğüm bir yanlış da, servis sağlayıcısının defer propertisinin false ayarlanmasına karşın provides metodu'nun da koyulmuş olmasıdır. Gerçi siz bu metoddan boş biz dizi döndürmüşsünüz ama provides metodu sadece defer özelliği true ayarlanmış ise implemente edilmek zorundadır.
4- Paket kurulumunda VideoApi aliasının ayarlanmasını öneriyorsunuz, ama servis sağlayıcınızdaki register metodunda da bu aliası ekletiyorsunuz.

Bir de küçük bir düzeltme gerekiyor: getVideoDetail metodunun döndürdüğü dizinin altıncı anahtarı Vimeo için upload_date iken youtube için uploaded'dir. Sanırım, bunların aynı olması gerekir.

Tekrar başarılar diliyorum.

sseffa

@sergin, önerileriniz için çok teşekkür ederim. Pakette dediğin gibi birçok hata olabilir, henüz tam istediğim şekle getiremedim, yazdıktan sonra birçok hata buldum bende. Bir taraftanda yazdığım CMS i geliştirmeye çalışıyorum bu paketi de direk olarak kullandığımdan zaman kaybı olmaması için çok fazla uğraşmak istemedim açıkçası ama dediğim gibi henüz çok eksiği var, bu söyledikleriniz ile birlikte en kısa sürede paketi güncelleyeceğim. Tekrardan teşekkür ederim.

Roni

Sergin hocam peki sınıfın bir özelliği static olabilir mi?

class Test{
 private static $status= true;
 public function info(){
 if(static::$status){
 .....
 }
 }

}

sergin

Propertiler de metodlar da static olabilir. Bir sınıfın static olup olmaması bence öneli değil. Laravel'de Facade sınıfları statictir, Eloquent Model sınıfı statictir, Str sınıfı statictir. Çok da kullanışlıdırlar. Benim dikkat çektiğim nokta, "ya Facade kullanmayın, ya da Facade kullanıyorsan gerçek sınıfınız static olmasın" demektir.

Roni

Hocam diyelim ki yukarıda verdiğim gibi sınıfın bir özelliği mecburen static olmak zorunda. Bu durum Facade için uygun olur mu? yani sadece özellik static olacak, diğer metotlar değil.
Not: ayrıca bu test etmede bir engel olarak görülebilir mi?

sergin

Bir sınıfta static olarak tanımlanmış bir metod tanımlanmış olması o sınıfı static yapmaya yetiyor, properti için de aynı şey geçerli diye biliyorum.

sseffa

Pakette gereken güncellemeleri yaptım, Facade ve Sınıf ismi aynı olduğundan hata veriyordu ama şuan sanırım çözdüm

Tekrar kontrol edebilirseniz çok sevinirim.

Aristona

Eline sağlık. Laravel paketleri konusunda pek iyi sayılmam ancak kodlarını incelerken birkaç şey gözüme çarptı.

1. https://github.com/sseffa/laravel-video-api/blob/master/src/Sseffa/VideoApi/VideoApiBase.php#L37 burada @ işareti kullanmışsın, @ kullanmak daima bad practice. Orada hangi hatayı susturmaya çalışıyorsan susturmak yerine çözüm bulmaya çalış.

2 https://github.com/sseffa/laravel-video-api/blob/master/src/Sseffa/VideoApi/VideoApiBase.php#L24, burada neden parseData diye bir fonksiyon oluşturdun? Aynı işi yapan (json_decode) fonksiyonlar üretmek kafa karışıklığına sebep olabilir. Gelecekte değiştirmeyi düşünüyor olabilirsin, ama bu durumda ben bir Parser sınıfı yazamayı tercih ederdim.

3. VideoApiBase traiti (sanırım tüm video sınıflarında olması şart bu traitin) yerine, bir abstract class olarak oluşturabilirdin. (ve bunu VideoApi sınıfında extend edebilirdin.) Bu durumda, oluşturduğun interfaceler daha uyumlu olur yazdığın video sınıflarıyla, çünkü şuan oluşturduğun interface ile yazdığın sınıfların bir ilgisi yok.

4. file_get_contents kullanmak yerine, bir Connector sınıfı yazmanı tavsiye ederim. file_get_contents yerine Curl gibi opsiyonlar var. Bunu bir sınıf yazarak çözersen daha kullanışlı olur. (veya default olarak Curl kullan, çünkü standart o sayılır)

5. parseData sınıfı altında, json_decode'ın döndüreceği sonuç kontrol edilmeli, yoksa veritabanına eklenirken syntax hataları oluşacak ve bir ihtimal yanlış data veritabanına girmiş olacak.

public function parseData($json) {
 $data = json_decode($json);

 if( json_last_error() === JSON_ERROR_NONE) 
 return $data;

 throw new \Exception("Invalid json.");
 }

sseffa

@aristona teşekkür ederim.

1. Orda biraz kolaya kaçtım evet, ilerde dediğin gibi curl'e geçirdiğimde o sıkıntı da ortadan kalkacaktır.

2. Sorunda fonksiyonel olsun diye öyle birşey yapmıştım ama zaten tek bir komut var olsa da olur olmasada olur. Dediğin gibi çok kapsamlı birşey olduğunda bir Parser yazılabilir. Ama şuan için çok gerekli değil gibi.

3. Sorunda ise ilk başta trait yoktu bir öneri ile ekledim sonradan abstract ile de yapılabilir. Açıkçası paket aceleye geldi çok değişiklik yaptım bir türlü oturtamadım sistemi. Bir taraftan da yazdığım CMS te kullanıyorum ondan çok fazla vakit harcayamadım.

4. file_get_contents yerine curl ilerisi için düşünülebilir. En kısa zamanda vakit bulursam bunu değiştireceğim.

5. son olarakta böyle bir kontrol yapılabili dediğin gibi çok teşekkür ederim tekrardan bir sonraki güncellemede bu dediklerini de dikkate alacağım

Roni

Sergin Abi, cevap için teşekkür ederim.

sergin

@sseffa, En iyi nasıldır sorusunun cevabı biraz kişiye göre değişir, çok güzel işler çıkaracağını bekliyorum. Genel olarak, yaptığımız işlerin kullandığımız framework'ün kullandığı ve önerdiği yöntemlere uygun olması daha iyi olur diyorum.
-İki başka sınıfta kullanılan metodları ayrı bir trait'te toplaman bence çok iyi olmuş ve bu Laravel'in son zamanlarda çok izlediği bir yol.
-Servis sağlayıcılarla ilgili sözümün tam tersini uygulamışsınız. Bir kez daha ifade edeyim: "bir servis sağlayıcısının defer özelliği false ise, bu servis sağlayıcısında provides metodu olmasına gerek yoktur. ". Olayı çok kısa özetlersem; Servis sağlayıcılar istek döngüsünün başlarında yüklenir ve register metodları çalıştırılır. Ama defer özelliği true ise register metodu hemen çalıştırılmaz. Bunun yerine provides metodundan dönen dizi elemanlarının işaret ettiği sınıflar gerekli olduğu zaman register işlemi yapılır. Bu yüzden, her istekte lazım olacak şeyler için defer özelliğini false yaparız, her istekte gerekli olmayabilen şeylerin defer özelliği true ayarlanır ki sadece bu şey bir istekte gerekli olursa yükleme yapılır.

sseffa

@sergin önerileriniz için çok teşekkür ederim bu paket benim için çok iyi bir deneyim oldu. Sizinde yardımlarınız ile çok daha kullanışlu hale getirmeye çalıştım. Bu provider konusunda bir çok pakete baktım hep aynı şekilde yapılmış bende yanlış okuduğumu düşünerek böyle yapmıştım

Trait kullanımım tam olarak doğru mu bilmiyorum, orda @aristona'nın da dediği gibi abstract bir sınıfta yapılabilirdi. Hangisi daha performanslı olurdu ya da hangisi daha doğru olurdu sanırım bu konu çok tartışılır. Açıkçası trait kullanımı sırasında interface de bir hata olabilir diye düşünmüştüm herhangi bir kullanım hatası çıkmayınca en son bu şekilde yapmaya karar verdim. Yine de önerileri olan arkadaşları da dinlemek isterim yanlışlarımı söylerlerse sevinirim.

Herkese teşekkür ederim tekrardan, iyi çalışmalar

sineld

Ellerine sağlık @sseffa, çalışmalarını takip ediyorum.
@sergin abinin katkılarıyla çok daha güzel olacak.

sseffa

teşekkür ederim @sineId

uzunca bir aradan sonra laravelde tekrardan birşeyler yapmaya çalışıyorum

sergin

@sseffa, Laravel'le gelen User modelinin eski ve yeni halini hatırlamak bir fikir verecektir. Eski halinde,

use Illuminate\Auth\UserInterface;
use Illuminate\Auth\Reminders\RemindableInterface;

class User extends Eloquent implements UserInterface, RemindableInterface {

Sınıfın içerisinde UserInterface ve RemindableInterface'in metodları implemente ediliyordu. Şimdiki halinde ise

use Illuminate\Auth\UserTrait;
use Illuminate\Auth\UserInterface;
use Illuminate\Auth\Reminders\RemindableTrait;
use Illuminate\Auth\Reminders\RemindableInterface;

class User extends Eloquent implements UserInterface, RemindableInterface {

	use UserTrait, RemindableTrait;

UserInterface ve RemindableInterface'in metodları artık sınıfın kendi içerisinde implemente edilmiyor. Bunun yerine, kullanmış olduğu UserTrait ve RemindableTrait traitlerinde implemente ediliyor. Laravel bu şekilde trait kullanımını birçok yerde kullanmaya başladı. Sizin trait buna çok uygun.

User sınıfının genişlettiği Eloquent (Asıl adı Model) sınıfı da abstract bir sınıftır.

Abstract bir sınıftan genişletme de yapabilirdiniz tabi ki, ama ben trait kullanmanızın uygun olduğu kanısındayım. Sadece isimlendirme geleneğine uyarak, bu traitin ismine VideoApiBase yerine VideoApiTrait demeniz daha uygun olurdu.

sseffa

@sergin tüm değişiklikleri yaptım fakat bu defer özelliği konusu biraz karıştı sanırım, son olarak defer özelliğini true yaptım ve gerektiğinde yüklenmesini sağladım sanırım bu benim paket için en uygunu değil mi?

Bunun yanında file_get_contents ile curl'un aynı anda kullanımını sağladım, oluşacak hatalar için önlem almaya çalıştım bu sefer sanırım biraz daha iyi oldu

Aristona

sergin yazdı

use Illuminate\Auth\UserTrait;
use Illuminate\Auth\UserInterface;
use Illuminate\Auth\Reminders\RemindableTrait;
use Illuminate\Auth\Reminders\RemindableInterface;

class User extends Eloquent implements UserInterface, RemindableInterface {

	use UserTrait, RemindableTrait;

Eskisine göre çok daha şık ve anlaşılabilir olmuş. Benim 3. maddede demek istediğim buydu. Açıkcası, şu durumda hangi yöntem daha iyi olur emin değilim.

sergin yazdı En iyi nasıldır sorusunun cevabı biraz kişiye göre değişir, çok güzel işler çıkaracağını bekliyorum. Genel olarak, yaptığımız işlerin kullandığımız framework'ün kullandığı ve önerdiği yöntemlere uygun olması daha iyi olur diyorum.

Kesinlikle. Ancak merak ettiğim bir konu var. Trait kullanmanın bu proje için klasik yöntemlere göre büyük artıları var mı? Laravel mesela neden trait kullanımına büyük önem vermeye başladı son zamanlarda? Klasik abstract class/DI mantığı neden yetersiz geliyor?

Şu yazıda http://blog.ircmaxell.com/2011/07/are-traits-new-eval.html Anthony Ferrara korkularından bahsetmiş. (yazı biraz eski) Bana da tecrübesiz geliştiricileri include günlerine geri döndürebilecek tehlikeli bir özellik gibi geliyor, ancak avantajları yönlerini öğrenmek isterim.

sergin

User sınıfının genişlettiği Eloquent (Asıl adı Model) sınıfı da abstract bir sınıftır.

derken işaret etmek istemiştim. Bir sınıf birden çok interface'i implemente edebilir ama sadece bir sınıfı extend edebileceği için bir metod sınıfın kendinde kodlanmamışsa, extend ettiği sınıfta kodlanmak durumunda. User'in implemente ettiği interface'lerin metodları User işlemleriyle ilgili olduğundan bu metodların Eloquent sınıfında implemente edilmesi gereksiz olacaktı. Ama bir sınıf birden çok trait'i kullanabilir ve Laravel bundan yararlanıyor, abstract bir sınıftan türetilmiş ve interface implemente eden sınıflarda trait kullanıyor.

Öte yandan, ne kadar gerekli sorusunun karşılığını ben de görebilmiş değilim. User'in kullandığı UserTrait ve RemindableTrait'in başka bir sınıf tarafından kullanıldığını görmedim (Laravel'deki diğer traitler de çoğunlukla tek bir sınıf tarafından kullanılıyor). Bir traitin birden çok sınıf tarafından kullanılması halinde daha işlevsel olacağını düşünüyorum.

Bu paket ile ilgili olarak ben YotubeApi ile VimeoApi sınıflarında birçok metod içeriğinin aynı olması nedeniyle, bunların VideoApi sınıfına alınmasının hatta iki sınıfın tümden kaldırılabileceğini söylemiştim:

VimeoApi ve YoutubeApi'nin birçok metodunun gövde kısımları birbirinin tıpatıp aynısı olduğuna göre bence doğrudan VideoApi sınıfında protected bazı özellikler ayarlanarak vimeo veya youtube tipleri ve id ayarlanabilir. Yine bazı protected metodlarla da kod tekrarlarında uzak durulabilir. Böylece rota tanımlamalarınız da
Route::get('video/{type}/{id}/{listveyadetay}', function (......... gibi olabilir.

sseffa, trait kullanmış. Laravel yolunun izlenmesi, üstelik bir trait'in birden çok sınıf tarafından kullanıldığını görmek hoşuma gitti.

Ama her iki sınıf bir abstract sınıftan da türetilebilirdi tabii ki.

Doğal olarak bir iş birçok şekilde yapılabilir. Böyle bir paket yazmam gerekseydi ben bir yapılandırma dosyasında, video API'larını, bunların url'larını, döndürülen json'dan nelerin alınacağını tanımlamak ve buradaki tanımları kullanarak işlem yapan genel bir sınıf kullanmak isterdim. Böylece yarın bir gün youtube ve vimeo dışında bir video API'sını da kullanmak istediğimde yapılandırma dosyasına eklemek yeterli olurdu.

Roni

@ssefa hocanın kodlarını aşağıdaki gibi kendi mantığıma göre değştirdim. Sefa hocadan izin almadım ama umarım anlayışla karşılar.

<?php
class VideoApi
{
 
 private $id;
 private $url;
 
 public function setUrl($url)
 {
 
 $this->url = $url;
 return $this;
 }
 
 public function getData($id)
 {
 
 try {
 $data = file_get_contents(str_replace('{id}', $id, $this->url));
 }
 catch (Exception $e) {
 throw new Exception($e->getMessage());
 }
 
 $data = json_decode($data);
 
 if (!is_array($data)) {
 return array(
 'Error' => "Video not found"
 );
 }
 return (array) $data[0];
 
 }
 
 public function getVideoDetail($id)
 {
 
 return $this->getData($id);
 }
 
 public function getVideoList($id)
 {
 return $this->getData($id);
 }
}




Route::get('video/vimeo/{id}', function ($id) {

 //$data = VideoApi::setUrl('http://vimeo.com/api/v2/video/{id}.json')->getVideoDetail($id); // video detail
 $data = VideoApi::setUrl('http://vimeo.com/api/v2/{id}/videos.json')->getVideoList($id); // video list

 print_r($data);
});


echo '<pre>';
$data = (new VideoApi())->setUrl('http://vimeo.com/api/v2/video/{id}.json')->getVideoDetail(101953117);

if(!isset($data['Error']))
print_r($data);

$data = (new VideoApi())->setUrl('http://vimeo.com/api/v2/{id}/videos.json')->getVideoList(670670);
echo '<pre>';
if(!isset($data['Error']))
print_r($data);
?>

Sonraki Sayfa »