lidıl lidıl reviewler

fatihergin commented 5 years ago

XML

xml root layoutlar frame layout olabilir -> eğer relative layout attributleri kullanılmayacaksa
componentlerin appcompat versionları tercih edilebilir -> Image view yerine appcompat image view gibi
image resource olarak mipmap klasörü bad practice. drawable kullanılmalı (android:src="@mipmap/chevron_left")
benzer componentler için en boy ölçüleri dimension olarak tanımlanıp kullanılabilir. örnekteki gibi hardcoded olmamalı android:layout_width="40dp" android:layout_height="40dp"
useless parent -> fragment_app_info_page sayfasında constraint layout kaldırılabilir. illa parent gerekiyorsa frame layout seçilmeli
10 tane constraint layout ? fragment_detailed_weather 1 tane constraint layout ile tümünün yaptığı iş yapılabilir. constraint layout çok yetenekli, ancak render maliyeti yüksek bir layout. az sayıda tercih edilmeli. zaten az sayıda kullanılarak her şeyi yapabilmesi esas çıkış noktası.
resim dosyaları doğrudan drawable altında olmamalı. uygun çözünürlükteki density klasörüne konulmalı(drawable-xxxhdpi gibi). drawable klasörüne konulanlar her cihazda farklı boyutta görünebilir.

JAVA

method isimleri Java'da küçük harfle başlamalı. (Bazı classlarda düzgün yapılmış) void LocationDbInserting(UUID id, Double latitude, Double longitude);
api key gibi sensitive datalar repoda yer almamalı. lokalde tutulup gradle ile build anında projeye eklenebilr. private static final String API_URL = "http://api.openweathermap.org/data/2.5/"; private static final String API_KEY = "2cd12ee50d586f06c24dbd1dd2cd8eca";
retrofit sorgularının sonuçları (parseCurrentWeatherResponse) manual parse edilmemeli. Bu da kabul edilebilir bir sonuç, ancak bu işin için converter factoryler var.
ismi DAO içeren bir classta retrofit çağrıları bad practice (WeatherDAO). bu class sadece database erişimi için kullanılmalı. retrofit yanıtında dönen obje bu modele dönüştürülecekse de mapper kullanılmalı.

dataaccessobjects'te kaldım, to be continued.

emreuskuplu commented 5 years ago

Yorum attığınız için teşekkür ederim, verdiğiniz feedbacklerin üzerinde çalışacağım.

emreuskuplu commented 5 years ago

Merhaba, verdiğiniz feedbackler için projeyi güncelledim.

Weather 1.2.3

XML

activity_detailed_bookmark_weather sayfasında relative layoutu gereksiz kullanmışım, layoutu merge(Frame Layout) olarak değiştirdim.
componentleri eski işletim sistemine sahip cihazlarla daha uyumlu çalışması için versiyonlarını appcompat olarak değiştirdim.
mipmap klasörün cihazın density bakımından bağımsız çalıştığını öğrendim, bu sebeple mipmap klasöründeki resim dosyalarını drawable klasörüne attım.
en boy ölçülerini dimens xml'ini oluşturup oraya attım.
fragment_app_info_page sayfasında gereksiz constraint layoutu kaldırdım.
constraint layoutları çok yapmamın sebepi view'leri gruplayarak ölçüm yapabilmekti. ancak chain ile herhangi bir viewgroup gerektirmeden ölçüm yapılabildiğini öğrendim, bu sepeble chain kullanarak gereksiz constraint layoutları kaldırdım.
resim dosyalarını density klasörlerine ayırdım.

JAVA

büyük harfle başlayan metod isimlerini değiştirdim ve tüm metodların kontrollerini yaptım.
bu proje için api keyleri kaldırmaya gerek duymadım ancak ileride bu uyarınızı dikkate alacağım. eğer bu projede api keylerin olması kötü bir izlenim yaratıyorsa keyleri kaldırabilirim. açıkçası uygulamayı test edecek kullanıcıların uygulamayı çalıştırması için api keyi almakla uğraşmalarını istemiyorum.
retrofit sorguların sonuçlarını converter factory ile parse ettim.
data acess object (DAO) classları sadece database erişimi için kullanıldığını bilmiyordum, bu dizayn hakkında araştırma yaptığımda diğer kalıcı datalara erişim sağlamak içinde kullanıldığını öğrenmiştim.
webservice klasörünü oluşturup retrofit çağrılarını bu klasörün içerisinden yaptım, ancak dao classlarını unit test yaptığım için retrofit çağıran classları dao classları tarafından çalıştırıyorum.
Mapper kullanarak dataları nasıl almam gerektiği konusunda internette pek örnek bulamadım, bu konu hakkında bana yardımcı olabilirseniz çok sevinirim.

emreuskuplu / Weather