Finding a better way to retrieve information about struct

Question

I've run into a problem about getting information about the structs. The code is written below:

pragma solidity ^0.4.23;
contract Transport {

    address public manager;
    Batch[] public batchArray;
    mapping(uint => bool) batchList;

    struct Batch {
        uint batchId;
        string batchCreationDate;
        address batchOwner;
        string batchOrigin;
        string productName;
    }
    constructor(address owner) public {
        manager = owner;
    }
    function createNewBatch(string batchCreationDate, address batchOwner, 
                         string batchOrigin, string productName) public { 

        uint batchId = uint(keccak256(block.difficulty, now));

        Batch memory newBatch = Batch ({
            batchId: batchId,
            batchCreationDate: batchCreationDate,
            batchOwner: batchOwner, 
            batchOrigin: batchOrigin,
            productName: productName
        });

        batchArray.push(newBatch);
        batchList[batchId] == true;
    }

    function getBatchDetails(uint batchId) public view returns(string, address, string, string) {
       //??? 
    }
}    

After I created a batch and put it into an array, I want to have a function of getting the details about a certain batch by providing batchId. I can't figure out what I need to input in getBatchDetails() function. Would appreciate your help.

By the way, do I overcomplicate this code? If so, any ideas on how I can restructure it?

Thank you

Answer 1

Yes. It's over-complicated and a little confused about organizing the data.

This sort of works, to show you a simple way to complete your function:

function getBatchDetails(uint row) public view returns(uint, string, address, string, string) {
   // require(batchList[batchId], "Batch not found.");, <== it is desirable to check, but not practical until the data organization is sorted out.
   // Let us carry on with returning the struct. We'll just assume we can find it by row number
   Batch storage b = batchArray[row]; 
   return(b.batchId, b.batchCreationDate, b.batchOwner, b.batchOrigin, b.productName);
}

I would reorganize the data with the Solidity CRUD pattern. https://medium.com/robhitchens/solidity-crud-part-1-824ffa69509a

Even better, use the library that implements the key management concerns: https://github.com/rob-Hitchens/UnorderedKeySet

And, use the Open Zeppelin Ownable contract for access control.

You get:

pragma solidity 0.5.1;

import "./Ownable.sol";
import "./HitchensUnorderedKeySet.sol";

contract Batch is Ownable {

    using HitchensUnorderedKeySetLib for HitchensUnorderedKeySetLib.Set;
    HitchensUnorderedKeySetLib.Set batchIds;

    struct BatchStruct {
        // uint batchId;   // (this is redundant)
        uint batchCreationDate; // (this should be a time number)
        address batchOwner;
        string batchOrigin; // should probably be an address
        string productName; // possibly a product id reference to another table
    }

    mapping(bytes32 => BatchStruct) public batches; // Use a bytes32 for primary key

    function genBatchId() internal view returns(bytes32) {
        return keccak256(abi.encodePacked(address(this), batchIds.count()));
    }

    function createBatch(address batchOwner, string memory batchOrigin, string memory productName) public onlyOwner returns(bytes32) {
        bytes32 id = genBatchId();
        batchIds.insert(id);
        BatchStruct storage b = batches[id];
        b.batchOwner = batchOwner;
        b.batchOrigin = batchOrigin;
        b.productName = productName;
        // emit ... should emit an event with the three inputs
    }

    // "public" at line 19 gives a less strict "free" getter: function batches(bytes32);

    function getBatch(bytes32 batchId) public view returns(uint, address, string memory, string memory) {
        require(batchIds.exists(batchId), "Batch not found");
        BatchStruct storage b = batches[batchId];
        return (b.batchCreationDate, b. batchOwner, b.batchOrigin, b.productName);
    }
}

Storage pointers in the return functions: https://blog.b9lab.com/storage-pointers-in-solidity-7dcfaa536089

As you press on, you will find the organization and KeySet library helps with other concerns such as enumerating the batchIds that exist. You could:

function getBatchCount() public view returns(uint) {
    return batchIds.count();
}

function getBatchIdAtIndex(uint index) public view returns(bytes32) {
    return batchIds.keyAtIndex(index); // this will revert if the request is out of bounds.
}

Here are the contracts I used to compile with solc 0.5.1 in Remix.

Ownable.sol

pragma solidity ^0.5.1;

/**
 * @title Ownable
 * @dev The Ownable contract has an owner address, and provides basic authorization control
 * functions, this simplifies the implementation of "user permissions".
 */
contract Ownable {

    address private _owner;

    event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);

    /**
     * @dev The Ownable constructor sets the original `owner` of the contract to the sender
     * account.
     */
    constructor () public {
        _owner = msg.sender;
        emit OwnershipTransferred(address(0), _owner);
    }

    /**
     * @return the address of the owner.
     */
    function owner() public view returns (address) {
        return _owner;
    }

    /**
     * @dev Throws if called by any account other than the owner.
     */
    modifier onlyOwner() {
        require(isOwner(), 
            "Ownable(101) - msg.sender is not the owner.");
        _;
    }

    /**
     * @return true if `msg.sender` is the owner of the contract.
     */
    function isOwner() public view returns (bool) {
        return msg.sender == _owner;
    }

    /**
     * @dev Allows the current owner to relinquish control of the contract.
     * It will not be possible to call the functions with the `onlyOwner`
     * modifier anymore.
     * @notice Renouncing ownership will leave the contract without an owner,
     * thereby removing any functionality that is only available to the owner.
     */
    function renounceOwnership() public onlyOwner {
        emit OwnershipTransferred(_owner, address(0));
        _owner = address(0);
    }

    /**
     * @dev Allows the current owner to transfer control of the contract to a newOwner.
     * @param newOwner The address to transfer ownership to.
     */
    function transferOwnership(address newOwner) public onlyOwner {
        _transferOwnership(newOwner);
    }

    /**
     * @dev Transfers control of the contract to a newOwner.
     * @param newOwner The address to transfer ownership to.
     */
    function _transferOwnership(address newOwner) internal {
        require(newOwner != address(0), 
            "Ownable(201) - newOwner cannot be 0x0");
        emit OwnershipTransferred(_owner, newOwner);
        _owner = newOwner;
    }
}

HitchensUnorderedKeySet.sol

pragma solidity 0.5.1; 

/* 
Hitchens UnorderedKeySet v0.93

Library for managing CRUD operations in dynamic key sets.

https://github.com/rob-Hitchens/UnorderedKeySet

Copyright (c), 2019, Rob Hitchens, the MIT License

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.

THIS SOFTWARE IS NOT TESTED OR AUDITED. DO NOT USE FOR PRODUCTION.
*/

library HitchensUnorderedKeySetLib {

    struct Set {
        mapping(bytes32 => uint) keyPointers;
        bytes32[] keyList;
    }

    function insert(Set storage self, bytes32 key) internal {
        require(key != 0x0, "UnorderedKeySet(100) - Key cannot be 0x0");
        require(!exists(self, key), "UnorderedKeySet(101) - Key already exists in the set.");
        self.keyPointers[key] = self.keyList.push(key)-1;
    }

    function remove(Set storage self, bytes32 key) internal {
        require(exists(self, key), "UnorderedKeySet(102) - Key does not exist in the set.");
        bytes32 keyToMove = self.keyList[count(self)-1];
        uint rowToReplace = self.keyPointers[key];
        self.keyPointers[keyToMove] = rowToReplace;
        self.keyList[rowToReplace] = keyToMove;
        delete self.keyPointers[key];
        self.keyList.length--;
    }

    function count(Set storage self) internal view returns(uint) {
        return(self.keyList.length);
    }

    function exists(Set storage self, bytes32 key) internal view returns(bool) {
        if(self.keyList.length == 0) return false;
        return self.keyList[self.keyPointers[key]] == key;
    }

    function keyAtIndex(Set storage self, uint index) internal view returns(bytes32) {
        return self.keyList[index];
    }

    function nukeSet(Set storage self) public {
        delete self.keyList;
    }
}

Hope it helps.

Answer 2

Just because the accepted answer seems overly complicated, I would simplify your code as follows:

1. Use the position in the array as the ID for each batch.
2. Shorten the struct field names. (No need to repeat "batch" over and over.)
3. Simplify createNewBatch by doing just a single push.
4. Get rid of getBatchDetails. There's already a getter because batches is public. Just call batches(0) to get the first batch, batches(1) to get the second, etc.
5. Drop batchList and manager which both appear to be unused in the current code.
6. Upgrade to Solidity 0.5.9.

Updated code:

pragma solidity 0.5.9;

contract Transport {
    struct Batch {
        string creationDate;
        address owner;
        string origin;
        string productName;
    }
    Batch[] public batches;

    function createNewBatch(
        string calldata creationDate,
        address owner,
        string calldata origin,
        string calldata productName
    )
        external
    {
        batches.push(Batch({
            creationDate: creationDate,
            owner: owner,
            origin: origin,
            productName: productName
        }));
    }
}