2
\$\begingroup\$

I have a code that adds a user to the database, getting the username from the dialog.My fragment implements the interface with the onInputSend method, which is called by the dialog when the user clicks the desired button. Here is the code that adds the user to the database. I want to hear criticism of my code.

UsersActivity

 public void onInputSend(@NonNull InputTextAlertDialog dialog, @NotNull String input) {
 userActionManager.isEmptyUser(input)
 .doOnSubscribe(disposable -> disposables.add(disposable))
 .subscribe(empty -> insertUser(dialog, input, empty));
}
private void insertUser(InputTextAlertDialog dialog, String userName, boolean isEmpty) {
 if (isEmpty) {
 if (!userName.trim().isEmpty()) {
 userActionManager.insertUser(new User(userName));
 dialog.cancel();
 } else {
 dialog.setErrorMessage(getString(R.string.user_not_have_name));
 }
 } else {
 dialog.setErrorMessage(getString(R.string.user_already_exists));
 }
}

InputTextAlertDialog

class InputTextAlertDialog(context: Context) : BaseAlertDialog(context) {
 var onInputOkListener: OnInputOkListener? = null
 private var input: EditText? = null
 private var error: TextView? = null
 var colorError: Int = Color.RED
 set(colorError: Int) {
 field = colorError
 error!!.setTextColor(colorError)
 }
 init {
 error = view.findViewById(R.id.error_text)
 input = view.findViewById(R.id.input)
 error!!.setTextColor(colorError)
 }
 fun setErrorMessage(errorMessage: String?) {
 error!!.text = errorMessage
 input!!.backgroundTintList = ColorStateList.valueOf(colorError);
 }
 override fun initOkClickListener() {
 ok.setOnClickListener { v: View? ->
 ok()
 }
 }
 override fun ok() {
 onInputOkListener?.onInputSend(this, input!!.text.toString())
 }
 override fun getLayout(): Int {
 return R.layout.input_text_layout
 }
 interface OnInputOkListener {
 fun onInputSend(dialog: InputTextAlertDialog, input: String)
 }
}

BaseAlertDialog

public abstract class BaseAlertDialog {
 AlertDialog alertDialog;
 protected Button cancel;
 protected Button ok;
 protected View view;
 private AlertDialog.Builder builder;
 protected Context context;
 BaseAlertDialog(Context context) {
 this.context = context;
 initDialog(context);
 initViews(view);
 initCancelClickListener();
 initOkClickListener();
 }
 private void initDialog(Context context) {
 if (getLayout() != 0) {
 view = LayoutInflater.from(context).inflate(getLayout(), null);
 } else {
 throw new RuntimeException("In getLayout () you should return the layout id");
 }
 builder = new AlertDialog.Builder(context);
 builder.setView(view);
 alertDialog = builder.create();
 }
 private void initViews(View view) {
 cancel = view.findViewById(R.id.cancel);
 ok = view.findViewById(R.id.ok);
 }
 private void initCancelClickListener() {
 cancel.setOnClickListener(v -> {
 alertDialog.cancel();
 });
 }
 protected void initOkClickListener() {
 if (ok != null) {
 ok.setOnClickListener(v -> {
 ok();
 alertDialog.cancel();
 });
 }
 }
 public void cancel(){
 alertDialog.cancel();
 }
 public void show(){
 alertDialog.show();
 }
 abstract void ok();
 abstract int getLayout();
}
asked May 6, 2020 at 11:03
\$\endgroup\$
5
  • \$\begingroup\$ I'm only here for the Kotlin... Would it be an error if the input or error would be absent? \$\endgroup\$ Commented May 6, 2020 at 13:47
  • \$\begingroup\$ Also it seems you're missing some code in UsersActivity... (the surrounding class eg.) \$\endgroup\$ Commented May 6, 2020 at 13:56
  • \$\begingroup\$ I left here only the code that interests me for verification.input NotNull he cannot be absent.What means error would be absent? \$\endgroup\$ Commented May 6, 2020 at 14:00
  • \$\begingroup\$ I mean should findViewById always find the view, or would it be ok if it didn't? \$\endgroup\$ Commented May 6, 2020 at 14:07
  • \$\begingroup\$ ok can not be found, cancel is required \$\endgroup\$ Commented May 6, 2020 at 15:37

1 Answer 1

1
\$\begingroup\$

Nullable

Your fields are optionals: you put a ? after the type.
This means every time you need to access them, you need to check if it is null...
If you want the fields to be always present, throw the NullPointerException when findViewById doesn't return a view.
Then you can make your field not nullable and you can remove all the (now) redundant checks...

oneline function

override fun getLayout(): Int {
 return R.layout.input_text_layout
}

This function doesn't need 3 lines of attention...
Change it to one:

override fun getLayout(): Int = R.layout.input_text_layout

you could remove the return-type as well:

override fun getLayout() = R.layout.input_text_layout

init

Personal choice:

I like to make everything as short as possible. This can be done by declaring the fields straight away instead of in the init block:

class InputTextAlertDialog(context: Context) : BaseAlertDialog(context) {
 var onInputOkListener: OnInputOkListener? = null 
 var colorError: Int = Color.RED
 set(colorError: Int) {
 field = colorError
 error!!.setTextColor(colorError)
 } 
 private var input: EditText = view.findViewById(R.id.input)!!
 private var error: TextView = view.findViewById(R.id.error_text)!!
 init {
 error!!.setTextColor(colorError)
 }
}

also

We can make the code above a bit simpler by using also:
also is a function which is called upon a variable (called the receiver).
also will return the receiver and accepts a lambda as parameter.
Inside that lambda, it gives one parameter: the receiver.
Therefor the following code:

class InputTextAlertDialog(context: Context) : BaseAlertDialog(context) {
 private var error: TextView = view.findViewById(R.id.error_text)!!
 init {
 error!!.setTextColor(colorError)
 }
}

can be reduced to:

class InputTextAlertDialog(context: Context) : BaseAlertDialog(context) {
 private var error: TextView = view.findViewById(R.id.error_text)!!
 .also{ v: TextView -> v.setTextColor(colorError) }
}

And if there is only one param, it can be accessed using it:

class InputTextAlertDialog(context: Context) : BaseAlertDialog(context) {
 private var error: TextView = view.findViewById(R.id.error_text)!!
 .also{ it.setTextColor(colorError) }
}
answered May 7, 2020 at 8:33
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.