You are viewing a single comment's thread from:

RE: Structure Backend Rework

in #utopian-io6 years ago

Thank you for your contribution! This looks like a big chunk of refactoring- need to add some tests to your new classes and code changes.

You might want to extract this into a variable i.e. the isBluePrintMissing may be costly to run twice. Also, I have seen thise appearing more than once - which hints a code smell.

You may be able to unroll the loop i.e put the if outside the for loop to achieve a better performance here.

I have seen code moving around and other bits of refactoring, as I have a bit concern that: does it break existing tests, if there are any?

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]

Sort:  

Thanks for the review, I was mainly changing some backend and then refactoring so I didn't want to focus on fixing code smells or performance issues which would be difficult to review.

It actually broke here and there some small things which we detected and fixed relatively quickly afterward.

In terms of tests, we have basically no unit tests at all with Structurize since most things are almost impossible to simulate as we would have to mock the entire Minecraft world.

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

Coin Marketplace

STEEM 0.16
TRX 0.15
JST 0.028
BTC 54358.77
ETH 2293.67
USDT 1.00
SBD 2.30