projectlombok / lombok

Very spicy additions to the Java programming language.
https://projectlombok.org/
Other
12.86k stars 2.38k forks source link

Add 'useSetters' configuration key to @XArgsConstructor #805

Open lombokissues opened 9 years ago

lombokissues commented 9 years ago

Migrated from Google Code (issue 770)

lombokissues commented 9 years ago

:bust_in_silhouette: alarmo.lord   :clock8: Jan 14, 2015 at 13:38 UTC

I kindly request you to add configuration key to @ RequiredArgsConstructor and @ AllArgsConstructor annotations.

lombok.toString.useSetters = [true | false] If set to true, lombok should use setters (if available) instead of accessing fields directly when generating constructors.

This will make possible the below code:

// Lombok import android.support.annotation.NonNull; import lombok.RequiredArgsConstructor;

@ RequiredArgsConstructor(useSetters = true) public class SimpleRecyclerViewAdapter extends RecyclerView.Adapter<RecyclerView.ViewHolder> {

@ NonNull private RecyclerView.Adapter mBaseAdapter;
/* Other fields */

private boolean mValid = true;

private void setBaseAdapter(final RecyclerView.Adapter baseAdapter) {
    mBaseAdapter = baseAdapter;
    mBaseAdapter.registerAdapterDataObserver(new RecyclerView.AdapterDataObserver() {
        @ Override
        public void onChanged() {
            mValid = baseAdapter.getItemCount() &gt; 0;
            notifyDataSetChanged();
        }

        /* Other overridden methods */

    });
}

}

Instead of:

// Vanilla Java public class SimpleRecyclerViewAdapter extends RecyclerView.Adapter<RecyclerView.ViewHolder> {

private RecyclerView.Adapter mBaseAdapter;
/* Other fields */

private boolean mValid = true;

public SimpleRecyclerViewAdapter(/* Required arguments */, RecyclerView.Adapter baseAdapter) {

    /* Simple initialization */
    mBaseAdapter = baseAdapter;
    mBaseAdapter.registerAdapterDataObserver(new RecyclerView.AdapterDataObserver() {
        @ Override
        public void onChanged() {
            mValid = mBaseAdapter.getItemCount() &gt; 0;
            notifyDataSetChanged();
        }

        /* Other overridden methods */

    });
}

}

Please, tell what do you think?

lombokissues commented 9 years ago

:bust_in_silhouette: alarmo.lord   :clock8: Jan 14, 2015 at 13:45 UTC

The idea behind this is to get rid off a simple initialization in constructor (like 'mField = field;') and retain only the meaningful parts.

lombokissues commented 9 years ago

:bust_in_silhouette: reinierz   :clock8: Jan 31, 2015 at 03:57 UTC

Not quite sure I get what you mean here. For every field where we wold have generated 'this.fieldName = argName', we should instead invoke 'setFieldName(argName);' if the setter exists?

Calling setters from the constructor is not exactly great coding style; the object hasn't actually been initialized fully yet. But it's a common enough pattern, I suppose.

lombokissues commented 9 years ago

:bust_in_silhouette: alarmo.lord   :clock8: Feb 02, 2015 at 14:09 UTC

"For every field where we wold have generated 'this.fieldName = argName', we should instead invoke 'setFieldName(argName);' if the setter exists?" Yes, that's what I mean.

There was many fields (in SimpleRecyclerViewAdapter), that could be simply initialized in a constructor and a single field (mBaseAdapter), that required a complex initialization process. Because of it, I couldn't use @ RequiredArgs annotation to hide this constructor.

"Calling setters from the constructor is not exactly great coding style" I understand. To prevent bad things from happening, we could check that the setter is 'final', and forbid usage of 'useSetters=true' if it's not. ( http://stackoverflow.com/questions/8501735/using-setter-methods-in-constructor-bad-practice )

lombokissues commented 9 years ago

End of migration