Problem while retrieving from array

Question

I have the following simple structure and array of that structure:

struct Document  {
      bytes32 ownerID;
      bytes32 documentID;
      bytes32 name;      
  }

  Document[] public documents;

I've a storeDoc() method that fills up the array:

function storeDocument (bytes32 _ownerID , bytes32 _documentID, bytes32 _name)  {
      Document memory newDoc;

      newDoc.ownerID = _ownerID;
      newDoc.documentID = _documentID;
      newDoc.name = _name;

      documents.push(newDoc);         

    }

And finally a getter. There could be multiple entries against one OwenerID, thus the memory arrays:

function getDocumentDetailsByID(bytes32 _ownerID) constant public returns (bytes32[], bytes32[], bytes32[]) {
      uint length = documents.length;

      if(registeredCandidates[_ownerID].clientAddress == msg.sender) {

          bytes32[] memory documentIDs = new bytes32[](length);
          bytes32[] memory names = new bytes32[](length);
          bytes32[] memory descriptions = new bytes32[](length);
          bytes32[] memory docYears = new bytes32[](length);


          for(uint i =0; i < length; i++) {

              if(documents[i].ownerID == _ownerID) {

                  Document memory currentDocument;
                  currentDocument = documents[i];

                  documentIDs[i] = currentDocument.documentID;
                  names[i] = currentDocument.name;

            }
          }


        return (documentIDs, names);  

      } else {
          return;
      }
  }

The problem I face is that only the first entry done by the storeDocument() returns using getDocumentDetailsByID() properly.

The second onward entries done by storeDocument() is coming as all zeros when I call the getDocumentDetailsByID() for the second time!!

Could someone please guide me what's I'm doing wrong? Thanks.

Answer 1

This contains an anti-pattern.

You can probably sense it growing in length and complexity. Even if the logic error is found (really not sure what the memory arrays are for) and resolved, you'll find you have still created a contract that will not scale.

The problem is your for loop iterates over the set. Each iteration costs a little gas. The cost will increase as the set grows. Eventually, this will collide with the block gas limit and the transactions will be unable to complete at all.

You can resolve this using more efficient internal structure that avoids unbounded loops. What you're doing closely resembles the Mapped Structs with Index pattern here: Are there well-solved and simple storage patterns for Solidity?

That one, or the variant with delete, will give you a scalable (same gas cost at any scale) internal storage pattern with random access by ID. If you don't need to enumerate the keys there is an even simpler pattern.

Hope it helps.