Also markS, read the feedback given by Kishore. I haven't written the same issues again in my feedback are quite important to resolve as well.

Hey Sharon, your contract is nice and I think it is already good enough for starters you figured out how to make an edit function as well. Good job there. Although you can definitely club those in a single function.

Besides that, you can just use events in your contract as a cherry on top. It will allow you to dynamically change your UI using react hooks such as useEffect(). They can be useful to maintain temporary logs as well. Events are triggered when specific functions are called.

You can read more about Solidity events in this link https://docs.soliditylang.org/en/v0.8.14/abi-spec.html?highlight=events#events

I have made a few events in your contract. Have a look at them in the PR linked below. Rest looks good to me.

It's a nice dapp DavidBut. Here are a few ways using which you can improve your contract for the users. First, if you wish that while deploying the contract only you should be the admin by default, environment variables should be the place where you put your address and import it in your contract. That's an alternative for your hardcoding approach. I have added a few events for functions like buy, addDrugs, change ownership as well. And, in the contract, I can add only 1 drug as an admin with a quantity of 1 by default. So, I did some edits so that the admin can now add a drug and also specify its quantity of the stock available in the store. This will get updated as users buy, and eventually a particular drug will go out of stock for that an event will be triggered. The price that the user has to pay will also be updated with respect to the quantity of a drug the user is buying. So I removed the isSold, we know that the drug is sold-out if we are out of stock. Also it was not clear that you are trying to transfer the ownership of a particular drug or the whole store. I have implemented the functionality for both of them. Transfer of ownership means like selling the rights for the product, not the buying as a consumer. Couple of keywords were missing and there was a logical issue in the payment of transfer of drug ownership function, all are rectified. Its a pretty good dapp and you can review the edits in the PR.

Hey Jaylukmann, this is an excellent dapp! I liked the idea of commission and that you also made an edit function. There are some places that I would recommend some changes.

1. First, you should use some events in your contract. It is always nice to have events in your dapp which will emit when the corresponding functions are executed. By this, you can maintain a temp log of the transactions and its main use is with React or JavaScript hooks that can be used to dynamically update the UI as soon as the transaction updates. It is done asynchronously so you don't have to refresh the whole webpage and re-request from all the view functions in the contract. I have made some events for your contract in the .sol file.

2. You have some functions where you have a condition to be satisfied before the execution of the function, this can alternatively be done by a "modifier" which is made for that. You can customise the function execution using a modifier as well.

3. It's great that you made an edit function in the dapp. Very few do that. Here is an improvement that you can do in your contract to reduce the code. In the "createArtWork" function you can check if there is already an Art existing in the "artworks" mapping for the address of "msg.sender". You have to give an index argument to the createArtWork function which you are already giving to the edit function. This will check if there is an already existing Art with the given index, if there is then it won't increment the counter "ArtworkLength" and the write function will overwrite the existing the Art with new data (basically that's your edit function), if the art does not exists for the given index, then the counter will increment and create a new entry.

4. In the art buying function, you have implemented that first the buyer pays the commission and then pays the art price to the owner so in this case if the payment to artise fails for some reason, the buyer paid the commission for nothing. The flow should be like the -> the artist should be paid first then the commission. Also we have to assign the new owner of the art now, which is the buyer. But this depends on your idea use-case.

You won't have to worry about the owner access this way as well, just enable the edit function to only those accounts who own the art work. Or the contract takes care of it.

In this way, you can essentially replace your whole edit function and still keep the functionality intact. Just keep in mind to pass correct arguments to the function createArtWork from the frontend.

The rest look good to me! The changes in the contract are in the PR linked below. If you have some difficulties you can comment on the PR to let me know.

Hi Great, this is a great dapp. I liked the idea behind determining how valuable the content can be. There are certain parts of the contract which can be improved. Like a user can like its own story many times and arbitrarily increase the amount of its story. And similarly, another user can like a particular story multiple times and increase the amount manyfold. My PR to your repo is below, in which a user can either like or dislike a story only once with the amount being adjusted, and the owner is not able to like its own post to make it fair. Also, keep in mind that you are mapping the storyLength with the Story struct, it should be fine as long as the user cannot delete a story. But, it is better if each story has its unique id. It is a cool dapp though. Thanks!

Hi Andy, this is a good dapp. You made a good contract as well. I did some changes to your contract that will work well with your current setup. I added some events which will trigger after some functions have been executed. Using those events you can use the "useEffect" JavaScript hook to directly update your DOM. An example here can be if a user deletes the billboard, update your DOM with the triggered event. Another thing, I saw that you used a "require" statement quite often to check if the function caller is the owner of the billboard or not. It will be better to use a "modifier" to do this as it reduces the repeatability of the code. Rest in your contract looks fine to me. Keep up the good work, thanks!

Hi davidekete. Its a nice dapp that you have created here. I see that you have added some new functions, in addition, to fit your use case here. There are some areas of improvement in your contract as I have stated here.

1. In your restock function, there is a bug that anyone can update the stock of any product. The ideal case should be that only the owner of that product should be allowed to restock the quantity of the product. So a require statement takes care of that.

2. You can make your buyProduct function more useful if you allow the users to buy a product of more than 1 in quantity. Currently the user can purchase only a single quantity of a particular product. The edit enables the user to buy more than 1. Rest of the things are looking fine in the contract. One good way to log the transactions of the contract is by using events. I have added the events in your contract for some functions. The purpose of such events is that, when a function is executed, that particular event will trigger with the log information of the function provided to it. Using this you can dynamically render in your frontend without reloading the complete website. This is useful as it can happen in real-time. Some events added are for when a product is added, when a product is sold, when a product is out of stock, and when a restock is done. The rest of the contract is fine. Good effort, and get a good understanding of how these additions work. For any doubts, you can comment on the pull request linked below.

Hey markS, the contract implementation looks quite solid. There is still some room for improvement there.

1. You should use the custom events in your contract and emit them whenever a corresponding function is executed. The advantage of this is that you can dynamically update your UI with the output of these events without reloading the whole page. Similar to what an asynchronous loading does. It also helps in keeping temporary logs on the transactions. I have added some events to the functions which will trigger every time someone calls it. You can read more about the events here https://docs.soliditylang.org/en/v0.8.14/structure-of-a-contract.html?highlight=events#events

2. There should be a way so that the user can only view those recipes that they have paid for. The function getMyRecipe() does that by returning an array containing the indexes of the recipes that the user has bought. I will advise you to take this array and loop it in your JavaScript code to call the getRecipe() function with the corresponding index as arguments to get purchased recipes.

The rest of the contract looks good to me. Just make some changes in the JavaScript as I told above to make it even better. You can refer to the changes in this PR. You can comment on the PR or here as well if you face doubts.

Hi josefsid, it is a nice idea to make a dapp for and I liked your UI. I went through your contract solidity and found it well made. But I do have some suggestions for improvement. I saw that you are using the goalReached flag variable in the Donate struct which is a bool type, I think it is a bit of redundant in the contract as it is not used much and simple logical operators do the job well. Then you have used the mapping donations to store the Donate struct posts, well here is the issue I see with that. In this type of dapp, your mapping is ever increasing as there is no way to remove a Donate post with that. So it will keep on increasing and practically increase the blocksize with each adding transaction. So it is better to use an Array here so that we can remove the posts which have already reached their goals. We can ensure that the blocksize is not always increasing that way. Another thing is I have added some events which are triggered when you transact respective functions, it will help you to get easy access of the transaction log in the frontend using React. Finally I have added a removeDonationPost function which controls the array size and can remove the posts which have reached their goals. I have added these changes in my pull request on Github, link provided below. Nice dapp and thanks!

Thanks for the feedback guys. It would be great if you all just go and contribute to the project. Make an Issue and a PR for any small thing you see and I'll look at and appreciate your contributions.

Hi Asapgrey, this is a nice dapp you made to incentivise the poems and a great way to like a poem. I went through your contract and added some code to it which you can see in the pull request I made in your repository. I added some events to be triggered in the contract when certain functions are executed such as "addPeom", "Like" and "buyPeom". Using these events you can render the changes immediately on your frontend. I also added the "require" statements to prevent some redundant transactions from happening for example there is no point in executing the "End_sale" function is the poem is already not for sale. On that I saw that if the user has taken the poem off the sale, it cannot be re-enabled for sale again. So, I added a function so that user can set the poem for sale again. But these functions are only to be performed by the owner of the poem and not by anyone, so a "modifier" takes care of that, only the owner of a poem can change if the poem should be on sale or not. All these changes are in the PR link below. Its a nice application, and with some modifications you can make the more efficient. Thanks!

Hey theodoreyip, it's a nice dapp. I went through the contract code and although it is written nicely, I did find some areas of improvement in the contract code and logic. Firstly, there are a few places where you use the datatype int256 for storing amount data, I think the datatype uint256 is more suitable here wherever the amount should be non-negative. Then you have a constructor where you are forwarding the cUSDTokenAddress from the frontend, it's better to make it as an internal variable and store it in the contract itself, instead of using a constructor. I couldn't find your ERC20 contract file in the GitHub repo, so I added the code to add the interface in the contract itself. It is better to add the keyword payable to the functions where you want to transfer the funds. Currently, the user can remove any amount from the contract treasury, this can be a security threat to the money pool. So I have implemented a logic that restricts the user to borrow only the amount of money that is less than or equal to 2x times the money in his/her existing account. Finally, a great way to improve the performance is to use the events in your contract, which you can directly use in the frontend with React. So you can dynamically change the treasuryamount or balance of the user in the frontend right after the add or remove functions are called. The added advantage here is you might not need to call the getBalance function every time you want to update it. I have attached my PR link below for you to have a look. Thanks!

Hi Jeffcole! You did a nice implementation of your smart contract with your frontend. There is still some room for improvement in your contracts. According to your code, we are assuming that the data in a block will always keep increasing as more people will add their flowers for sale. It will be ever-increasing as there is no way to delete them. That might increase your gas consumption as your mapping increases in size because you do use a for loop to iterate over the entire mapping to get all of the flowers listed on your dapp. My suggestion here is to use an array instead of mapping and add a unique id for each flower posted in the struct. Using arrays we can effectively control how many are listed. I implemented a delete function in the contract to do that. You can delete a flower if you want with a given id. I have added a binary search function in the contract to find the index of the flower in the array with the given id. Using that index you can get or delete a flower struct. So now you have a way to decrement blocksize. Another word of caution here is that the binary search function is a pure type function, so call it from the react frontend that is externally and it will not require gas to execute. If you execute it internally it will require gas to iterate over the array. Further steps on how to do that are in the comments in the contract itself. Unlike mapping, the array will readjust the size and reduce the storage required. Finally, I have added some more events which will be triggered when certain functions are executed and conditions are met. My pull request is here below. Nonetheless, it is a great dapp.

Hey Oliem, you have made a nice dapp. The review given by princeibs above me does cover some good improvements in the contract. I would still highlight the way the like and dislike functions are executed and the changes I made. You can definitely use "arrays" to keep a track of likes and dislikes in the Car struct for each individual NFT, but every time you want to search for a specific car you will be required to run a linear search algorithm through out the array, which will be time consuming as the array will grow as more people list. You can use mapping to directly index with the senders' address and the index of the Car to check if this particular is liked or disliked. I have used a nested mapping for that, which also resolves the problem where a user can like and dislike a car at the same time. In addition to that I have just added some events to be triggered when specific functions are executed so that you can easily render them on the frontend. It will help you learn about events if you don't currently know about them. There is an instance where you used the "storage" keyword to get a car, the "memory" keyword should work just fine with less gas and should not persist even after the function execution. You can checkout the code on my PR linked here to understand the changes. Rest is good, keep up the learning!