Programadores Android, NO hagáis esto


#1

Me he encontrado este código en un blog profesional de android el cual no citaré.

 public void onClick(View view) {
        if (view == findViewById(R.id.btnSave)){
            StudentRepo repo = new StudentRepo(this);
            Student student = new Student();
            student.age= Integer.parseInt(editTextAge.getText().toString());
            student.email=editTextEmail.getText().toString();
            student.name=editTextName.getText().toString();
            student.student_ID=_Student_Id;
 
            if (_Student_Id==0){
               _Student_Id = repo.insert(student);
 
                Toast.makeText(this,"New Student Insert",Toast.LENGTH_SHORT).show();
            }else{
 
                repo.update(student);
                Toast.makeText(this,"Student Record updated",Toast.LENGTH_SHORT).show();
            }
        }else if (view== findViewById(R.id.btnDelete)){
            StudentRepo repo = new StudentRepo(this);
            repo.delete(_Student_Id);
            Toast.makeText(this, "Student Record Deleted", Toast.LENGTH_SHORT);
            finish();
        }else if (view== findViewById(R.id.btnClose)){
            finish();
        }

No hagáis lo que hace Teo.


#2

Solo puedo decir que me recuerda a esto…

public void ActionPerformed(Event e){
//Chorrotera de ifs....
}

Nunca he visto algo más feo que ese método en java…


#3

Pero en el Action no tenías ningún identificador tipo ID… Pero ahí lo está usando, y usa el findViewById que es jodidamente pesado.


#4

Perdonad que reviva un hilo de hace casi un año. Pero no entiendo una cosa.

Esta claro que el churro de ifs es mas feo que un frigorífico por detrás, pero lo que comentas de findViewById , no se, en los tutoriales que estoy haciendo lo usan a porrillo.

¿Acaso no es una buena práctica?


#5

No es problema usarlo en el onCreate o de vez en cuando, lo que no puede es usarse «todo el rato», igual que en los ListView se usan ViewHolder para no tener que hacer findById cada vez que se actualiza el contenido.

El código de arriba se podría haber escrito así:

 public void onClick(View view) {

   switch(view.getId()) {
       case R.id.btnSave:
            StudentRepo repo = new StudentRepo(this);
            Student student = new Student();
            student.age= Integer.parseInt(editTextAge.getText().toString());
            student.email=editTextEmail.getText().toString();
            student.name=editTextName.getText().toString();
            student.student_ID=_Student_Id;
            if (_Student_Id==0){
               _Student_Id = repo.insert(student);
 
                Toast.makeText(this,"New Student Insert",Toast.LENGTH_SHORT).show();
            }else{
 
                repo.update(student);
                Toast.makeText(this,"Student Record updated",Toast.LENGTH_SHORT).show();
            }
          break;
         case R.id.btnDelete:
            StudentRepo repo = new StudentRepo(this);
            repo.delete(_Student_Id);
            Toast.makeText(this, "Student Record Deleted", Toast.LENGTH_SHORT);
            finish();
            break;
         case R.id.btnClose:
            finish();
        }

Mucho más limpio, eficaz y escalable.


#6

Muchas gracias por la info y por la refactorización @MiguelAngelLV me ha sido de bastante utilidad porque ahora mismo estaba liado con los ListView :bow:


#7

Siento decirte que los ListView ahora no se usan, se sustituyeron hace cosa de dos años por los RecyclerView, mucho más eficientes y polivalentes