simon987 / Much-Assembly-Required

Assembly programming game
https://muchassemblyrequired.com
GNU General Public License v3.0
925 stars 87 forks source link

Refactor ParseInstruction #215

Closed AstronautEVA closed 1 year ago

AstronautEVA commented 4 years ago

I feel like it's still a little complex but I'm not sure what else to modify. Let me know of any ideas!

kevinramharak commented 4 years ago

This helps the readability a lot. I think maintenance can be improved by using arrays with the mnemonics and joining them when creating the regex. Personally i prefer caching the regex object instead of creating a new instance everytime but that maybe a bit overkill as performance does not seem a problem. Just a preference in style.

instead of

RegExp('\\b(?:mov|add|sub|and|or|test|cmp|shl|shr|xor|rol|ror|sal|sar|rcl|xchg|rcr)\\b').test(mnemonic.toLowerCase());

Do something like this:

const mnemonics = ['mov', 'add', 'sub', ...];
const regex = new RegExp(`\\b(?:${mnemonics.join('|')})\\b`);
function someCheckWithRegex(code) {
   return regex.test(code.toLowerCase())
}

Ofcourse depending on which of my suggestions you would want to implement.

AstronautEVA commented 4 years ago

I completely agree! I had planned on doing something about that but then got side-tracked. I'll push some updates later.