11
\$\begingroup\$

I write the following version of an if-statement very often. And every time, I hate the way that I make it, but I don't know how to improve it.

For example, I have two ArrayList's. If one of them is not empty, then I convert it to a StringBuilder and send it per mail. The problem is, first I check if one of them is not empty to know if I should send a mail, and then I check it again to know which of them is not empty. Example code:

if(!array1.isEmpty() || !array2.isEmpty()){
   // I need to distinguish the StringBuilder
   StringBuilder array1string = new StringBuilder();
   StringBuilder array2string = new StringBuilder();
   // so one of them is not empty. but which? now I must double check it
   if(!array1.isEmpty()){
      for(String string : array1){
         array1string.append(string);
      }
   }
   if(!array2.isEmpty()){
      for(String string : array2){
         array2string.append(string);
      }
   }
   mail.sendMail("Array1: " + array1string == null?"-":array1string.toString() 
      + ", Array2: " + array2string == null?"-":array2string.toString());
}

So is there a way to prevent this double check?

\$\endgroup\$
  • 3
    \$\begingroup\$ array1string and array2string will never be equal to null; you're giving them each a value near the start of the code segment. They might result in empty strings, but they're not null. \$\endgroup\$ – Brian S Nov 18 '13 at 15:21
  • \$\begingroup\$ @dTDesign: Do you know that the mail that is sent might still indicate that both arrays are empty? \$\endgroup\$ – quamrana Nov 20 '13 at 11:34
13
\$\begingroup\$

Your code can be shortened to:

if(!array1.isEmpty() || !array2.isEmpty()) {
    mail.sendMail("Array1: " + getContent(array1) + ", Array2: " + getContent(array2));
}

When you have:

String getContent(ArrayList<String> list) {
    if(!list.isEmpty()) {
        StringBuilder sb = new StringBuilder();
        for(String string : list) {
            sb.append(string);
        }
        return sb.toString();
    }
    return "-";
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ You mean if (!array.isEmpty()) \$\endgroup\$ – ChrisWue Nov 18 '13 at 8:44
  • \$\begingroup\$ This is why I love this site. very elegant way. Thanks! \$\endgroup\$ – Michael Schmidt Nov 18 '13 at 9:02
  • \$\begingroup\$ I think the whole code block inside getArrayContent method can be substituted with return array.isEmpty() ? "-" : Arrays.toString(array); \$\endgroup\$ – Anirban Nag 'tintinmj' Nov 18 '13 at 10:58
  • 1
    \$\begingroup\$ @tintinmj Arrays.toString() adds ,, [ and ] to the output, something that OP does not want. \$\endgroup\$ – Adam Siemion Nov 18 '13 at 11:18
  • 2
    \$\begingroup\$ The OP said they were ArrayLists, not arrays. String[] should be List<String>, and the method can actually be built with generics to support any kind of list. I'll add an answer to show how. \$\endgroup\$ – Simon Forsberg Nov 18 '13 at 12:33
11
\$\begingroup\$

Adam's answer is good, but I just couldn't resist modifying his method a bit:

<E> String getArrayContent(Collection<E> list) {
    if(list.isEmpty())
        return "-";

    StringBuilder sb = new StringBuilder();
    for(E value : list) {
        sb.append(value);
    }
    return sb.toString();
}

This method has the following modifications compared to Adam's:

  • It uses Collection because you stated that you were using ArrayList in the question. (And this method works with any type of collection, not only List or ArrayList). Collection is the superinterface which declares both isEmpty() and extends Iterable
  • It uses generics so it can be used for any kind of collection, not only Collection<String>
  • I prefer to return early to simplify readability and not have to indent code more than necessary

If you also want a method for regular arrays, that method could call the method for collections by converting the array to a list:

<E> String getArrayContent(E[] array) {
    return getArrayContent(Arrays.asList(array));
}
| improve this answer | |
\$\endgroup\$

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.