0
\$\begingroup\$

My code below is working fine but I know it can be done more efficiently using a loop.

I'm checking the VAL (such as description, ingredient2, etc.) which are Strings from my SQL lite Db to see if they are Null or Empty. If true I hide them in addition to the divider views in my layout. If false I update the EditText field (such as description_text_view) with the String value from the Db.

    override fun onLoadFinished(loader: Loader<Cursor>, cursor:  Cursor?) {

    if (cursor == null || cursor.count < 1) {
        return
    }

    // Proceed with moving to the first row of the cursor and reading data from it
    // (This should be the only row in the cursor)
    if (cursor.moveToFirst()) {
        // Find the columns of recipe attributes that we're interested in and extract out
        // the value from the Cursor for the given column index
        val name = cursor.getString(cursor.getColumnIndex(DataEntry.COLUMN_RECIPE_NAME))
        val description = cursor.getString(cursor.getColumnIndex(DataEntry.COLUMN_RECIPE_DESCRIPTION))
        val glass = cursor.getString(cursor.getColumnIndex(DataEntry.COLUMN_RECIPE_GLASS))
        val ingredient1 = cursor.getString(cursor.getColumnIndex(DataEntry.COLUMN_RECIPE_INGREDIENT_1))
        val ingredient2 = cursor.getString(cursor.getColumnIndex(DataEntry.COLUMN_RECIPE_INGREDIENT_2))
        val ingredient3 = cursor.getString(cursor.getColumnIndex(DataEntry.COLUMN_RECIPE_INGREDIENT_3))
        val ingredient4 = cursor.getString(cursor.getColumnIndex(DataEntry.COLUMN_RECIPE_INGREDIENT_4))
        val ingredient5 = cursor.getString(cursor.getColumnIndex(DataEntry.COLUMN_RECIPE_INGREDIENT_5))
        val ingredient6 = cursor.getString(cursor.getColumnIndex(DataEntry.COLUMN_RECIPE_INGREDIENT_6))
        val ingredient7 = cursor.getString(cursor.getColumnIndex(DataEntry.COLUMN_RECIPE_INGREDIENT_7))
        val instructions = cursor.getString(cursor.getColumnIndex(DataEntry.COLUMN_RECIPE_INSTRUCTIONS))


        // Required fields
        // Update the views on the screen with the values from the database
        recipe_title.text = name
        glass_type.text = glass
        ingredient_text_view_1.text = ingredient1


        // Optional fields
        // If any of the following fields are empty, hide in the layout.
        // The if statements below work but there has to be a cleaner way. TODO
        if (description.isNullOrEmpty()) {
            description_text_view.visibility = View.GONE
         } else description_text_view.text = description

        if (ingredient2.isNullOrEmpty()) {
            ingredient_text_view_2.visibility = View.GONE
            divider2.visibility = View.GONE
            divider3.visibility = View.GONE
        } else ingredient_text_view_2.text = ingredient2

        if (ingredient3.isNullOrEmpty()) {
            ingredient_text_view_3.visibility = View.GONE
            divider3.visibility = View.GONE
            divider4.visibility = View.GONE
        } else ingredient_text_view_3.text = ingredient3

        if (ingredient4.isNullOrEmpty()) {
            ingredient_text_view_4.visibility = View.GONE
            divider4.visibility = View.GONE
            divider5.visibility = View.GONE
        } else ingredient_text_view_4.text = ingredient4

        if (ingredient5.isNullOrEmpty()) {
            ingredient_text_view_5.visibility = View.GONE
            divider5.visibility = View.GONE
            divider6.visibility = View.GONE
        } else ingredient_text_view_5.text = ingredient5

        if (ingredient6.isNullOrEmpty()) {
            ingredient_text_view_6.visibility = View.GONE
            divider6.visibility = View.GONE
            divider7.visibility = View.GONE
        } else ingredient_text_view_6.text = ingredient6

        if (ingredient7.isNullOrEmpty()) {
            ingredient_text_view_7.visibility = View.GONE
            divider7.visibility = View.GONE
            divider8.visibility = View.GONE
        } else ingredient_text_view_7.text = ingredient7

        if (instructions.isNullOrEmpty()) {
            instructions_title.visibility = View.GONE
            instructions_text_view.visibility = View.GONE
            divider8.visibility = View.GONE
            divider9.visibility = View.GONE
        } else instructions_text_view.text = instructions
    }
}

Using the suggestions from gervais.b I was able to setup a HashMap which cleans up the TextViews. Is there a way to clean up the dividers as well?

Improved code that cleans up the textviews but not the dividers.

 val isPopulated = HashMap<String, TextView>()
        isPopulated.put(description, description_text_view)
        isPopulated.put(ingredient2, ingredient_text_view_2)

        for ((string, textView) in isPopulated) {
            if (string.isEmpty()) {
                textView.visibility = View.GONE
            } else {
                textView.text = string
            }
        }
\$\endgroup\$
1
\$\begingroup\$

If all your objects (description, ingredient) share a common type then you can add them to a collection and loop on it. However you don't knwo which text_view is linked to it. So you should add them to a map. I suppose that description and ingredients are String, then you can do something like

Map<String, TextView> viewsByString = new HashMap<>();
viewsByString.put(description, description_text_view);
viewsByString.put(ingredient2, ingredient2_text_view);
// ..
for (Entry<String, TextView> entry : viewsByString) {
   String string = entry.getKey();
   TextView view = entry.getValue();
   if ( string.isNullOrEmpty() ) {
        view.visibility = View.GONE;
    } else {
       view.text = description;
    }
}

However, you have to populate the map and that is not always clean. But you can still reduce your code by extracting the condition to a method:

void bind(String text, TextView view) {
    if ( text.isNullOrEmpty() ) {
        view.visibility = View.GONE;
    } else {
        view.text = text;
    }
}

// ...
bind(description, description_text_view);
bind(ingredient2, ingredient2_text_view);

Or try to use the reflection API, but that's not welcome on Android

Edit, show composite

This is not valid Kotlin, but I hope you can convert it.

class TextWithDividers {
    TextView text;
     Divider[] divs;
      // Constructor 
     setText(String text) {
         this.text.text = text
     }
     setVisibility(int v) {
       this.text.visibility = v;
       this.divs.foreach { d => d.visibility=v }
     }
}
\$\endgroup\$
  • \$\begingroup\$ Thank you for your advice. That's an interesting idea and I'll explore that. I also added the complete method along with some recent changes. Hopefully it more clear as to what I'm trying to accomplish. \$\endgroup\$ – Dallas Sep 13 '17 at 22:44
  • \$\begingroup\$ Regarding your whole code. I'll opt for the dedicated method solution. Or maybe an object that hold a refernce to your activity and cursor : new Binder(this, cursor), binder.set(DataEntry.COLUMN_RECIPE_NAME, recipe_title) and binder.setOrHide(DataEntry.COLUMN_RECIPE_DESCRIPTION, description) \$\endgroup\$ – gervais.b Sep 14 '17 at 7:32
  • \$\begingroup\$ gervais.b I added some comments to my original question based on your feedback. I think we are close but the dividers are now the tricky part. Any ideas? \$\endgroup\$ – Dallas Sep 14 '17 at 19:09
  • \$\begingroup\$ You can make a composite that contains the ˋTextViewˋ and the dividers. This composite will set the visibility to each of his components and the text to the ˋTextViewˋ \$\endgroup\$ – gervais.b Sep 14 '17 at 19:59
  • \$\begingroup\$ See my edit. But please, learn how to Google. Being able to learn and discover new things is a key for a developer \$\endgroup\$ – gervais.b Sep 15 '17 at 7:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.