RE: Minecolonies & Small Fixes and Features

[justyy]

Thank you for your contribution.

1. I can see you add null checks and try to solve/fix the errors, but it is very dangerous, as you are explicitly hiding the errors. One better way would be to throw the exception and log it in upper application level.
2. Some paramters to method could be replaced with interfaces, so that you can unit tests them later with mocking.
3. public, static methods can be unit tested for example the checkForListInInvAndRequest especially you have made quite a lot of changes which need to be covered by tests.

Your contribution has been evaluated according to Utopian policies and guidelines, as well as a predefined set of questions pertaining to the category.

To view those questions and the relevant answers related to your post, click here.

---

Need help? Chat with us on Discord.

[utopian-moderator]

Comments

[raycoms]

Thanks for the evaluation.

1. Crashing is always the worst thing to do in our environment. Since it's a game, players don't care about side effects, but they do care about crashing and waiting another 10 minutes for modding Minecraft to boot up.
   Usually, we either add logging where we don't expect nulls to happen at all. In some places, where it can happen due to the code flow, we don't.
   I agree with the unit testing part, but, this is not so easy since mocking Minecraft is not an easy part at all. When we extend our API we plan on improving the way we handle Minecraft libraries to make mocking much easier.

[utopian-io]

Thank you for your review, @justyy! Keep up the good work!