Wątek przeniesiony 2020-10-27 15:49 z Java przez somekind.

Czy to dobry fragment kodu z wykorzystaniem Android MVP?

0

Uczę się właśnie MVP dla Androida. Zastanawiam się czy to co napłodziłem jest poprawne w kontekście tego wzorca czy jeszcze coś można udoskonalić, a może jest całkowicie do bani:

public interface CredentialsContract {

    interface Model {
        interface OnFinishedLoginListener {
            void onFinishedLogin(String body);
            void onFailureLogin(Throwable t);
        }
        void checkLogin(OnFinishedLoginListener onFinishedListener, String login, String pass);
    }


    interface View {
        void showBigProgressDialog();
        void hideBigProgressDialog();
        void setPropertiesWhenLoginIsTrue();
        void setPropertiesWhenLoginIsFalse();
        void setLoginTextViewText(String response);
    }


    interface Presenter {
        void saveSomeCredentials(String nameLogin, String passLogin);
        void saveEmptyCredentials();
        void checkLoginPresenter(String login, String pass);
    }
}


public class RestData implements CredentialsContract.Model {

    @Override
    public void checkLogin(final OnFinishedLoginListener onFinishedListener, String login, String passWhenLogin) {
        Call<String> result = Api.getClient().checkLogin(login, passWhenLogin);
        result.enqueue(new Callback<String>() {
            @Override
            public void onResponse(Call<String> call, Response<String> response) {
                    onFinishedListener.onFinished(response.body());      
            }

            @Override
            public void onFailure(Call<String> call, Throwable t) {
                Log.e("tag", t.toString());
                onFinishedListener.onFailure(t);
            }
        });
    }
    
}

public class CredentialsPresenter implements CredentialsContract.Presenter, CredentialsContract.Model.OnFinishedLoginListener {

    CredentialsContract.Model model;
    CredentialsContract.View view;
    SharedPreferences sharedPreferences;
    SharedPreferences.Editor editor;

    public CredentialsPresenter(CredentialsContract.View view, SharedPreferences sharedPreferences) {
        model = new RestData();
        this.view = view;
        this.sharedPreferences = sharedPreferences;
        editor = sharedPreferences.edit();
    }

    @Override
    public void saveSomeCredentials(String nameLogin, String passLogin) {
        editor.putString("userLogin", nameLogin);
        editor.putString("userPassword", passLogin);
        editor.putBoolean("cbSaveCredentials", true);
        editor.apply();
    }

    @Override
    public void saveEmptyCredentials() {
        editor.putString("userLogin", "");
        editor.putString("userPassword", "");
        editor.putBoolean("cbSaveCredentials", false);
        editor.apply();
    }

    @Override
    public void checkLoginPresenter(String login, String passWhenLogin) {
        view.showBigProgressDialog();
        model.checkLogin(this, login, passWhenLogin);
    }


    @Override
    public void onFinishedLogin(String body) {
        view.setLoginTextViewText(body);
        if(body.contains("true"))
            view.setPropertiesWhenLoginIsTrue();
        if(body.contains("false"))
            view.setPropertiesWhenLoginIsFalse();
        view.hideBigProgressDialog();
    }

    @Override
    public void onFailureLogin(Throwable t) {
        view.hideBigProgressDialog();
    }



public class Credentials extends AppCompatActivity implements CredentialsContract.View {

    private EditText etNameLogin, etPassLogin;
    private CheckBox cbSaveCredentials, cbTerms;
    private TextView tvLoginStatus, tvRegistrationStatus;
    private ProgressBar pbBig;
    private CredentialsPresenter credentialsPresenter;
    private Button bOk;

    @Override
    protected void onCreate(@Nullable Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.credentials_layout);

        SharedPreferences pref = getApplicationContext().getSharedPreferences("Preferences", 0); // 0 - for private mode
        credentialsPresenter = new CredentialsPresenter(this, pref);

        initUIFields(pref);

        findViewById(R.id.tvTerms).setOnClickListener(v -> initRegulationsDialog());

        bOk = findViewById(R.id.bOk);
        bOk.setOnClickListener(v -> {
            if (cbSaveCredentials.isChecked())
                credentialsPresenter.saveSomeCredentials(etNameLogin.getText().toString(), etPassLogin.getText().toString());
            else
                credentialsPresenter.saveEmptyCredentials();

            Intent i = new Intent(getApplicationContext(), Gra.class);
            i.putExtra("userLogin", etNameLogin.getText().toString());
            i.putExtra("onlineGame", true);
            startActivity(i);
        });

        findViewById(R.id.bLogin).setOnClickListener(v -> credentialsPresenter.checkLoginPresenter(etNameLogin.getText().toString(), etPassLogin.getText().toString()));

    }

    public void initUIFields(SharedPreferences pref) {
        etNameLogin = findViewById(R.id.etNameLogin);
        etNameLogin.setText(pref.getString("userLogin", ""));
        etPassLogin = findViewById(R.id.etPassLogin);
        etPassLogin.setText(pref.getString("userPassword", ""));
        cbSaveCredentials = findViewById(R.id.cbSaveCredentials);
        cbSaveCredentials.setChecked(pref.getBoolean("cbSaveCredentials", false));
        etNameRegistration = findViewById(R.id.etName);
        etEmailRegistration = findViewById(R.id.etEmailRegistration);
        etPassRegistration = findViewById(R.id.etPassRegistration);
        etPassConfirmationRegistration = findViewById(R.id.etPassConfirmationRegistration);
        cbTerms = findViewById(R.id.cbTerms);
        tvLoginStatus = findViewById(R.id.tvLoginStatus);
        tvRegistrationStatus = findViewById(R.id.tvRegistrationStatus);
        pbBig = findViewById(R.id.bigProgressBar);
    }

    public void initRegulationsDialog() {
        AlertDialog.Builder dialogRegulations = new AlertDialog.Builder(Credentials.this);
        dialogRegulations.setTitle("Terms and conditions");
        dialogRegulations.setMessage("Content");
        dialogRegulations.setCancelable(false);
        dialogRegulations.setNeutralButton("OK", (dialog1, which) -> dialog1.dismiss());
        dialogRegulations.show();
    }

    @Override
    public void showBigProgressDialog() {
        pbBig.setVisibility(View.VISIBLE);
    }

    @Override
    public void hideBigProgressDialog() {
        pbBig.setVisibility(View.INVISIBLE);
    }

    @Override
    public void setPropertiesWhenLoginIsTrue() {
        bOk.setEnabled(true);
        tvLoginStatus.setTextColor(ContextCompat.getColor(this, R.color.green));
    }

    @Override
    public void setPropertiesWhenLoginIsFalse() {
        bOk.setEnabled(false);
        tvLoginStatus.setTextColor(ContextCompat.getColor(this, R.color.red));
    }

    @Override
    public void setLoginTextViewText(String response) {
        tvLoginStatus.setText(response);
    }
}



1
  • Ustaw kolokowanie składni dla Javy bo nic nie widać.
  • Jeśli to rzeczywiście kod dla Androida to zła kategoria. Powinna być Mobilne. Nie każdy programista Javy zna Androida.
1

Pierwszy artykuł wypluty przez Googla jasno stawia sprawę, że grzebanie w SharedPreferences powinno być w modelu, a u Ciebie prezenter to robi.

0

Dlaczego nie mogę edytować treści tematu?

0

"Dzwonią dzwony, ale nie wiadomo w którym kościele" - będę mieć temat na oku. Jeśli ktoś wcześniej nie wrzuci dobrych podpowiedzi podzielę się nimi z Tobą bez zbędnej zwłoki - w tym tygodniu.

0
lubie_programowac napisał(a):

"Dzwonią dzwony, ale nie wiadomo w którym kościele" - będę mieć temat na oku. Jeśli ktoś wcześniej nie wrzuci dobrych podpowiedzi podzielę się nimi z Tobą bez zbędnej zwłoki - w tym tygodniu.

Dużo jest źle?

2
  1. W MVP Model, View oraz Presenter powinny być agnostyczne. Nie powinieneś mieć tam referencji do androida. U Ciebie jest to zrealizowane połowicznie: nie masz referencji do klas androidowych ale w nazwach metod już tak: (dodatkowe pytanie dlaczego jest big w nazwie? Moze byc jakis small? Jesli nawet tak to te metody powinny miec odzwierciedlenie w domenie a nie w wielkosci czyli np full screen progress albo header progress)

void showBigProgressDialog(); -> void showProgress()
void hideBigProgressDialog(); -> void hideProgress()

  1. Polecam zapoznać się z architekturą VIPER która jest używana na iOSie ale robi dobrą robotę na androidzie VIPER ~= mVP + clean architecture + routing. m z małem po model wtedy składa się z usecasów. Pownieneś mieć ~LoginUseCase który w konstruktorze przyjmowałby ~LoginService oraz TokenService. W LoginService miałbyś zawarty LoginRepository który pobierałby dane z internetu oraz je parsował. W prezenterze nie powinno być referencji do androida.

  2. Dodaj do projektu ViewBinding - pozbędziesz się masy kodu z findviewbyid.

  3. W androidzie nie stosuje się notacji węgierskiej -> private EditText etNameLogin, etPassLogin; -> loginName, loginPassword, private ProgressBar pbBig; -> progress

  4. Widok powinien być pasywny, najlepiej gdybyś kolor przesyłał już w prezentera a nie decydował w View czy error to red albo brown a loginSuccess to green a może magenta.

  5. 0 for private mode == Context.MODE_PRIVATE

  6. W widoku nie powinieneś sprawdzać co się dzieje w sharedpreferences, jest to odpowiedzialność prezentera ( a w tym use casów) do obsługi logiki biznesowej i poinformowaniu widoku jak ma się zachować. Tak samo dla cbSaveCredentials.isChecked()

  7. Dodaj statyczne finalne klasy do stringów bo teraz np "userLogin" masz w dwóch miejscach.

  8. Obsługa "userLogin" i rodziny pewnie powinna być w klasie ~ UserRepository z implementacją lokalną która byłaby używana przez wspomniany LoginService oraz wyżej LoginUsecase

  9. Wprowadzenie daggera / innego frameworku do DI (toothpick, Hilt teraz popularny jest) jest dobrym pomysłem

  10. Mógłbyś dodać prostą imeplementację dla rxjavy, ona dobrze się spina z Single<T> z retrofitem oraz ~VIPERem. Korzystałem polecam.

  11. Oczywiście kotlin oraz korutyny też są warte rozważenia.

Tutaj masz dobry widok jak wygląda VIPER https://cdn.ttgtmedia.com/rms/onlineimages/whatis-viper_desktop.png

Ogólnie kod nie jest zły. Kontrakt MVP mi się podoba bo jest angostyczny - bez referencji androida. Reszta to de facto kilka spraw + jakiś cukier składniowy do ogarnięcia w kilka h.

Jakbyś miał jakieś pytania pisz, odpowiem w wolnej chwili.

0

@lubie_programowac: dzięki za sugestie. Strasznie to wszystko skompliwane i dużo tego ;/ odnośnie 1, tak, występuje progressbar niemal na pół ekranu :) odnośnie punktu 7, Kolega wyżej pisał, że jest to odpowiedzialność modelu, tak sam na tej stronie którą załączył Ty piszesz że jednak prezentera.

0
Kubaz napisał(a):

@lubie_programowac: dzięki za sugestie. Strasznie to wszystko skompliwane i dużo tego ;/ odnośnie 1, tak, występuje progressbar niemal na pół ekranu :) odnośnie punktu 7, Kolega wyżej pisał, że jest to odpowiedzialność modelu, tak sam na tej stronie którą załączył Ty piszesz że jednak prezentera.

My z @cs mówimy o tym samym. Według @cs shared preferences powinno być w modelu - ja się z tym zgadzam. Dla mnie model to Interactors / Usecases + services + repositories + mappers + entities. W punkcie 7 napisałem: jest to odpowiedzialność prezentera ( a w tym use casów) do obsługi logiki biznesowej i poinformowaniu widoku jak ma się zachować . Prezenter powinien gadać z usecasami które w środku gdzieś tam powinny mieć sharedpreferences. Po pobraniu danych z usecasow powinien on przekazac je do widoku.

0

@lubie_programowac: Skoro SharedPrefs powinno być w modelu, wtedy będę potrzebował context w modelu, a skoro tak, to przy tworzeniu nowego obiektu modelu w konstruktorze presentera też będę potrzebował context, bo ten presenter korzysta z modelu. A przecież presenter nie powinien być platform dependent jak więc ten problem rozwiązać?

0

Zastosowałem taki myk jak sugeruje gościu w odpowiedzi: https://stackoverflow.com/questions/39100105/need-context-in-model-in-mvp.

0

W dużym uproszczeniu, prezenter wie co należy zrobić, ale nie wie jak i gdzie. Od tego ma właśnie widok i model. Przy tworzeniu modelu możesz przekazać do niego context w konstruktorze, tak jak i przy tworzeniu prezentera powinieneś podać w konstruktorze z jakiego modelu on korzysta, a u Ciebie prezenter sam sobie tworzy model i dlatego musisz w prezenterze dostać się do context'u. Tworzenie modelu powinno być w onCreate, potem utworzenie prezentera:

  Model model = new RestModel(prefs);
  Presenter presenter = new CredentialsPresenter(this, model);

Oczywiście to wymaga przemeblowania interfejsu modelu, żeby zawierał metody do zapis, odczytu itd.

2
lubie_programowac napisał(a):
  1. Widok powinien być pasywny, najlepiej gdybyś kolor przesyłał już w prezentera a nie decydował w View czy error to red albo brown a loginSuccess to green a może magenta.

Chyba jedyny punkt, z którym się nie zgodzę. O takich detalach lepiej kiedy decyduje widok. Jeśli zmienię sposób reprezentacji danych, to będę musiał orać w prezenterze. Osobiście wolę takie rzeczy załatwiać zwykłą flagą, interfejsami, enumami albo sealed class. Chociaż ciężko jest czasem znaleźć balans.

Jestem jeszcze sceptycznie nastawiony do use-case-manii, ale to grubszy temat, bo nie mam nic przeciwko use-casom jako tako, tylko często robi się przez to degeneracja projektu z wstrzykiwaniem 20 use-casów do klas. Ale jakiś podstawowy klocek realizujący logikę biznesową jest potrzebny.

0

To jak jesteśmy już przy clean codzie w Presenterach, to ja nie przepadam za tym patternem. Powoduje konieczność tworzenia interfejsów, które najczęściej mają tylko 1 implementację. Jak dla mnie to zbędny boiltercode. Nie widziałem by ktoś unitowo testował zmianę tekstu czy kolorów w UI. Dodatkowo Presenter ma niepotrzebnie referencję do widoku. Dlatego wolę ViewModel i przekazywanie do widoku danych przez LiveData, gdzie ViewModel nigdy nie ma referencji do widoku. Widok ma referencję do ViewModelu, ale stara się być tak bardzo "głupi" jak tylko się da.

1

Mój prezenter zazwyczaj wygląda tak. Najczęściej raczej implementuje jakiś interfejs, ale to dla polimorfizmu a nie dla ukrycia implementacji.

class Presenter {
  fun emitUiModels(events: Flow<Event>): Flow<UiModel> = TODO()
}

ViewModel to raczej kwestia nomenklatury, którą Google zaczął promować wraz z pracą nad Jetpackiem. Yigit z resztą gdzieś na Reddicie pisał, że niefortunnie dobrali nazwę. Już na długo przed ich bibliotekami robiło się obserwowanie bez referencji do widoku za pomocą callbacków, event busów czy RxJavy. https://cashapp.github.io/2020-06-09/android-presenters

1 użytkowników online, w tym zalogowanych: 0, gości: 1