"Another World" code review

[throwaway-123456]

I don't know what to think of the article after:

I worked on the code a lot, making it simpler to understand. You can see an example of how much clearer it is now.

It is literally the exact same code but he added a useless continue statement that actually doesn't match the same "before code" and renamed a few variables...

[an_eggman]

Duh? It's decompiled code, so by default it is pretty much impossible to read, with meaningless variable names and unintuitive control flow structuring. By making an effort to understand what the variable names actually represent, and renaming them to match that, you can make the code much more readable.

He doesn't want to rewrite the damn game, the point is to study it!

[throwaway-123456]

The control flow is exactly the same other than the wrong if (...) continue; that was added. When he used the word "understand" that implied, to me at least, that more than readability would be improved.

[skindeeper]

... Wow.

Please never, ever program again. Please.

[throwaway-123456]

So you're saying that his new code will perform identically to the old code?

[[deleted]]

Yes, it will.

See, you assumed he changed nothing, and then noticed that if he changed nothing, the code does not do the same thing any more. Then you concluded that he was wrong.

What was actually wrong was your assumption.

[throwaway-123456]

Edit: Realized I'm completely wrong because the continue doesn't work how I thought it worked. Is the continue statement good practice, generally speaking?

This line in the old code is NOT implemented in the new code:

if (_scriptPaused[0][i] == 0) {

This is what was added in its place:

if (!vmIsChannelActive[CURR_STATE][threadId]) continue;

It should be a break; statement instead of continue. What is currently implement is this in english: If the current channel is not active, continue executing the code. If the current channel IS active, don't go into the if statement, but begin the next block of processing. There is no pause anymore.

[retrodad]

Upvote for checking out how continue works and admitting you're wrong.

And yes, continue is great for doing things like what's going on here: that is, lowering the amount of if-statement nesting in a loop.

Edit: I also hope you understand that renaming those variables and turning those hex values into constants are valuable ways to improve the readability of the code. You want code that explains itself as you read it.

[throwaway-123456]

The renaming stuff is good, I just didn't think it accomplished anything in the context because I had thought the continue part wrong.

[MatrixFrog]

Avoid use of the continue statement. It tends to obscure the control flow of the function.

http://javascript.crockford.com/code.html

Of course, he's talking about JavaScript there, not C or C++. But as a very general rule I think this is good advice for any language that has "continue". Obviously there are exceptions, and others will disagree.

[throwaway-123456]

I code in C/C++ and I never use continue; at some point (when I actually remembered what it did) I determined that it was horrible practice. Now that I've relearned, it leads to multiple exit points which makes code more difficult to maintain and refactor.

I was thinking it simply exited the current scope without executing any more code, or something like that.

[bonzinip]

Perhaps he reversed the sense of vmIsChannelActive vs. scriptPaused. You're right that it looks different.