Skip to content

Created a new constructor for SubTextItem #67

Open
wants to merge 2 commits into from

3 participants

@leandrob02

It's now possible to enable a SubTextItem on GDListActivity.

@cyrilmottier

Why are you using a Boolean here and not a boolean ? You're creating a useless object (autoboxing) that may force the GC to clean it.

Owner

I'm not a huge fan of having large constructors ... Could you please give me a reason why you don't like doing things like :

SubtextItem item = new SubtextItem();
item.text = "blabla";
item.subtext = "blabla";
item.enable = true;
@leandrob02

Okay, am I wrong in using Boolean and not boolean, but why you set enabled false in the constructor?

@nicholas-jordan

// in abstract base class boolean enabled Will be autoboxed as code is written
// Consider Boolean Object as that is what we will end up with anyway
// Great attention to intended design here now to avoid forced re-factoring after much design
// Compiler will do this anyway but without the default ~ which results in enabled being a null

public Boolean enabled = Boolean.valueOf( false / true );

TextItem(String text) {
// okay, most do not put call to super() in code
// changing the nullary constructor in Item as shown directly above
// we do it here so that enabled is non-null
// null checking here to handle both cases
// really reduces issues later in Exception Chaining
this.text = ((text == null)? "" : text ); // looks kludgy now but oh, wow later
}

@cyrilmottier
Owner

@leandrob02: The original purpose of a SubtextItem is to display a title with a subtitle displayed on several lines. When originally developing it, I considered this item should be used to give a description of something. As a result, it is intentionally disabled by default. On the contrary, a SubtitleItem is enabled by default.

Please also note you can create your own items if you really want to. The process is pretty simple and consists on copying what's GreenDroid is doing internally.

@nicholas-jordan: Sorry but I don't understand your comment in this leandrob02's pull request. We are not using generics nor abstract classes (which by the way has nothing to do with autoboxing)? Concerning the null checking I don't agree with this as a null is different from an empty string. It is up to the developer to handle both cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.