/**
@dev internal function for adding new item to market
* See {addItemToMarket}
*/
function _addItemToMarket(
uint8 _saleType,
uint256 _askingPrice,
uint256 _newItemId,
address _seller,
TokenData memory _tokenData
) internal virtual {
// adding data to itemsForSale struct.
itemsForSale.push(
SaleOrder(
false,
false,
_saleType,
_askingPrice,
_newItemId,
payable(_seller),
_tokenData
)
);
// adding Item to active list.
activeItems[_tokenData.tokenAddress][_tokenData.tokenId] = true;
// checking if requested tokenId is same as that in the Sale Data
require(itemsForSale[_newItemId].id == _newItemId, "Item id mismatch");
// emit item added event.
emit itemAdded(
_newItemId,
_tokenData.tokenId,
_askingPrice,
_tokenData.royalty,
_tokenData.tokenAddress,
_tokenData.creator
);
}
This line in the function is unnecessary: require(itemsForSale[_newItemId].id == _newItemId, "Item id mismatch");
That line is unnecessary because in the two places that _addItemToMarket is called itemsForSale.length is passed in as the _newItemId. Because of this the require statement will always be true so there is no need to check it.
To make the logic of _addItemToMarket a little clearer I suggest removing the uint256 _newItemId parameter and returning the new item ID.
Here is the suggested change:
/**
@dev internal function for adding new item to market
* See {addItemToMarket}
*/
function _addItemToMarket(
uint8 _saleType,
uint256 _askingPrice,
address _seller,
TokenData memory _tokenData
) internal virtual returns (uint256) {
//getting new Id for the Item.
uint256 newItemId = itemsForSale.length;
// adding data to itemsForSale struct.
itemsForSale.push(
SaleOrder(
false,
false,
_saleType,
_askingPrice,
_newItemId,
payable(_seller),
_tokenData
)
);
// adding Item to active list.
activeItems[_tokenData.tokenAddress][_tokenData.tokenId] = true;
// emit item added event.
emit itemAdded(
_newItemId,
_tokenData.tokenId,
_askingPrice,
_tokenData.royalty,
_tokenData.tokenAddress,
_tokenData.creator
);
return newItemId;
}
This is the
_addItemToMarket
function:This line in the function is unnecessary:
require(itemsForSale[_newItemId].id == _newItemId, "Item id mismatch");
That line is unnecessary because in the two places that
_addItemToMarket
is calleditemsForSale.length
is passed in as the_newItemId
. Because of this the require statement will always be true so there is no need to check it.To make the logic of
_addItemToMarket
a little clearer I suggest removing theuint256 _newItemId
parameter and returning the new item ID.Here is the suggested change: