tags:

views:

184

answers:

7

I have a method which returns a document list consisting of all these attributes:

private String activitySource, activitySystemStatus, regionCode, channel,
               creatorUserId, displayOnAccessIndicator, documentLocationCode,
               extraDetails, keywordList, lastUserId, summaryText,
               textTypeIndicator;

Here's the method:

public DocumentSummary[] getDocumentList(String crn, String dateFormat) {
    log.debug("Entering getDocuments() method");
    //
    RetrieveDocumentListRequestDTO requestDto =
            new RetrieveDocumentListRequestDTO();
    RetrieveDocumentListResultDTO resultDto =
            odrUtils.retrieveDocumentList(crn, requestDto);
    Document[] dtoDocuments = resultDto.getDocumentArray();
    //
    DocumentSummary[] domainDocuments =
            new DocumentSummary[dtoDocuments.length];
    //
    for (int count = 0; count < domainDocuments.length; count++) {
        domainDocuments[count] =
                new DocumentSummary(dtoDocuments[count], dateFormat);
    }
    return domainDocuments;
}

Instead of returning a whole list, how can I just return only a list with displayOnAccessIndicator is set?
Any suggestions or sample code would be appreciated.

+1  A: 

I assume the attribute displayOnAccessIndicator is of Document class. You can try something like this, there could be some syntax mistake so please ignore those.

List domainDocuments = new ArrayList();
for (int count = 0; count < dtoDocuments.length; count++) {
     if (dtoDocuments[count].isDisplayOnAccessIndicator()) {
          domainDocuments.add(new DocumentSummary(dtoDocuments[count], dateFormat));
     }
}

return domainDocuments.toArray(new DocumentSummary[0])
Bhushan
Use a generic list List<DocumentSummary> instead and you may optimize by passing an array of right length to toArray(new DocumentSummary[domainDocuments .size()]).
Arne Burmeister
One minor correction here: the for loop needs to run to `dtoDocuments.length`. Vivia La used the result array for the loop.
rudolfson
Arne the size is not known as list gets populated based on condition, and nowhere user mentioned he is using 1.5 or above so the solution is based on his code.
Bhushan
rudolfson thanks for pointing the mistake
Bhushan
+4  A: 

I would swap all this code:

    Document[] dtoDocuments = resultDto.getDocumentArray();

    DocumentSummary[] domainDocuments = new DocumentSummary[dtoDocuments.length];

    for (int count = 0; count < domainDocuments.length; count++) {
            domainDocuments[count] = new DocumentSummary(dtoDocuments[count], dateFormat);
    }

For this (some is personal taste):

List<DocumentSummary> arr = new ArrayList<DocumentSummary>();

for (Document doc : resultDto.getDocumentArray())
    if (doc.displayOnAccessIndicator)
        arr.add(doc);
Oli
One minor correction here: the for loop needs to use `dtoDocuments`. Vivia La used the result array for the loop
rudolfson
Or even better, I'll just hook onto resultDto and remove another line of code. Good catch though.
Oli
A: 

it depends. if you mean from the function you put above, simply replace:

domainDocuments[count] = new DocumentSummary(dtoDocuments[count], dateFormat);

with:

if (!string.IsNullOrEmpty(dtoDocuments[count].displayOnAccessIndicator))
    domainDocuments[count] = new DocumentSummary(dtoDocuments[count], dateFormat);
Luke Schafer
I believe the code is Java, not C#. :-)
JesperE
Ugh. this will lead to sparse array which I believe isn't what the question author wanted.
Ran Biron
'ugh' ... He said that's what he wanted... he only wanted only those items... As if you'd waste rep on a devote for your own mistakeAlso, didn't realise it's java :)
Luke Schafer
A: 

First, you should really use a generic list instead of an array, if possible. If you can't change the method signature, I'd still use a generic list to build up the contents of the list and then convert the generic list to an array using Collections.toArray().

JesperE
A: 

If I get you right you would like to add only those elements to the resulting array, which have document.displayOnAccessIndicator == true. Since this could be easily checked for in the for-loop I think your problem lies in handling the result array.

I would highly recommend the usage of the java collection interfaces (e.g. java.util.List). I'm assuming at least Java version 5 in this example.

Document[] dtoDocuments = resultDto.getDocumentArray();
List<DocumentSummary> domainDocuments = new ArrayList<DocumentSummary>();

for (Document doc : dtoDocuments) {
    if (doc.isDisplayOnAccessIndicator()) { // or doc.getDisplayOnAccessIndicator()
        domainDocuments.add(new DocumentSummary(dtoDocuments[count], dateFormat));
    }
}

// change return type accordingly
// or use return domainDocuments.toArray(new DocumentSummary[domainDocuments.size()]);
return domainDocuments;
rudolfson
A: 

If you want to use lists, see one of the suggestions above me (personally I like Oli's best). You can improve performance by using the ArrayList constructor like this:

... = new ArrayList<DocumentSummary>(
            dtoDocuments.length * DISPLAY_ON_ACCESS_PREDICTION);

where DISPLAY_ON_ACCESS_PREDICTION is either:

  • 1 (int), if your result sets aren't huge and you don't care for a little extra memory being taken.
  • A constant, based on your knowledge of the system.
  • If performance is really important - a variable based on history (average, moving average, etc) or prediction (if you can make one ahead of time).

Also, you could use the ArrayList.trimToSize() if the model is very big and memory is something you wish to conserve.

If you want to stay with arrays, here are two suggestions:
Suggestion 1, assuming getting the asking the indicator status is "slow" (time/resources consuming):

//...
DocumentSummary[] domainDocuments = new DocumentSummary[dtoDocuments.length];
int skipped = 0;
for (int count = 0; count < domainDocuments.length; ++count) {
    if (doc.isDisplayOnAccessIndicator()) {
        DocumentSummary summary = new DocumentSummary(dtoDocuments[count], dateFormat);
        domainDocuments[count - skipped] = summary;
    } else {
        ++skipped;
    }
}
//trim the array
DocumentSummary[] result = new DocumentSummary[count - skipped];
System.arrayCopy(domainDocuments, 0, result, 0, count - skipped);
return result;

Suggestion 2, assuming getting the asking the indicator status is "fast" (time / low on resources):

int actualCount = 0;
for (int count = 0; count < domainDocuments.length; ++count) {
    if (doc.isDisplayOnAccessIndicator()) {
        ++actualCount;
    } else {
        ++skipped;
    }
}
DocumentSummary[] domainDocuments = new DocumentSummary[actualCount];
int skipped = 0;
for (int count = 0; count < domainDocuments.length; ++count) {
    if (doc.isDisplayOnAccessIndicator()) {
        DocumentSummary summary = new DocumentSummary(dtoDocuments[count], dateFormat);
        domainDocuments[count - skipped] = summary;
    } else {
        ++skipped;
    }
}
return domainDocuments;
Ran Biron
A: 

Instead of using arrays, I would use ArrayList (or more generally Collection) and then filter based on a predicate. see Java: What is the best way to filter a Collection

kenj0418