5
\$\begingroup\$

I have below Java code to compare two list values with some condition:

 public boolean compareLists(List<SortData> prevList, List<SortData> modelList) {
        if (prevList != null && modelList != null && prevList.size() == modelList.size()) {
            boolean indicator = false;

            for (SortData modelListdata : modelList) {
                if (prevList.size() > 1 && modelList.size() > 1 && prevList.size() == modelList.size()) {
                    for (SortData prevListdata : prevList) {
                        if (modelListdata.getListBoxHeaderName().equals(prevListdata.getListBoxHeaderName())) {
                            indicator = false;
                            break;
                        }
                        else{
                            indicator = true;
                        }
                    }
                }
                for (SortData prevListdata : prevList) {
                    if (prevListdata.getListBoxHeaderName() != null && modelListdata.getListBoxHeaderName() != null) {
                        if (prevList.size() == 1 && modelList.size() == 1
                                && !prevListdata.getListBoxHeaderName().equals(modelListdata.getListBoxHeaderName())) {
                            return true;
                        } else if (prevListdata.getListBoxHeaderName().equals(modelListdata.getListBoxHeaderName())
                                && prevListdata.isSortAscending() != modelListdata.isSortAscending()) {
                            return true;
                        } else if (modelListdata.getListBoxHeaderName().equals(prevListdata.getListBoxHeaderName())
                                && modelListdata.getPosition() != prevListdata.getPosition()) {
                            return true;
                        }  
                    }
                }
            }

            if (indicator) {
                return true;
            }
        } else {
            return true;
        }

        return false;

    }

But I do not think this code is optimized.

The condition I am checking:

Both List not null
If both List size 1 & Name not null return true
If header Names equals but sortAcscending boolean variable not same return true.
If header Names equals but position a integer value not same return true
If both List size > 1 and bth list size equals then check same header name exist in both the List if not then return true.

I don't think the above code is optimized and I have to write two for loops which look ugly. What can I do to optimize this code?

Improve this question
\$\endgroup\$
2
  • 1
    \$\begingroup\$ If you could, please state more concretely what this method is intended to do. Rather than just restating your program logic with words, explain what its purpose is. Right now it just looks like a hodgepodge of things you're checking and the code itself is pretty unclear (and, I think, has a bug for what is right now condition #5 in the OP corresponding to inner loop #1). \$\endgroup\$ – Radiodef Dec 24 '13 at 6:32
  • \$\begingroup\$ That i changed please see edited question \$\endgroup\$ – subodh Dec 24 '13 at 7:47
1
\$\begingroup\$

Some comments:

&& modelList.size() > 1

this check is redundant, in fact

if (prevList.size() > 1 && modelList.size() > 1 && prevList.size() == modelList.size()) {

this whole check doesn't depend on the for variable, and can be hoisted outside the for.

The big if that starts at the first line can be inverted to reduce nesting.

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.