Refactor run.rs to reduce repetition and use proper Error types

src/run.rs

@@ -4,10 +4,23 @@ use crate::{
     memory::{self, Memory},
     register::{self, Registers},
 };
+use thiserror::Error;
 
 mod instructions;
 mod parse;
 
+/// `Error` represents a run error. Typically these will be fatal and panicking on their emission
+/// is likely the best course of action
+#[derive(Error, Debug)]
+enum Error {
+    /// An invalid address was provided to an instruction that required an immediate address
+    #[error("invalid immediate address: {0:X}")]
+    InvalidImmediateAddress(usize),
+    /// An invalid address was stored in the given register and attempted to be loaded to/from
+    #[error("invalid address stored in register ({0}): {1:X}")]
+    InvalidRegisterAddress(register::Combined, usize),
+}
+
 #[derive(Debug, Default, Clone)]
 pub struct Processor {
     num_cycles: u64,
@@ -15,6 +28,13 @@ pub struct Processor {
     memory: Memory,
 }
 
+macro_rules! assert_ok {
+    ($result: expr) => {
+        let res: Result<_, _> = $result;
+        assert!(res.is_ok(), "{}", res.unwrap_err());
+    };
+}
+
 impl Processor {
     fn run(&mut self, instruction: &RunnableInstruction) {
         match instruction.instruction {
@@ -28,55 +48,29 @@ impl Processor {
             }
 
             Instruction::LDFromRegisterAddress { src, dst } => {
-                let src_address = self.registers.get_combined_register(src);
+                let load_res = self.load_from_register_address_to_register(dst, src);
-                let memory_value = self
+                assert_ok!(load_res);
-                    .memory
-                    .get(src_address.into())
-                    .unwrap_or_else(|| panic!("invalid value stored in ({src}): {src_address:X}"));
-
-                self.registers.set_single_register(dst, memory_value);
             }
 
             Instruction::LDFromImmediateAddress { src_address, dst } => {
-                let memory_value = self
+                let load_res = self.load_from_address_to_register(dst, src_address.into());
-                    .memory
+                assert_ok!(load_res);
-                    .get(src_address.into())
-                    .unwrap_or_else(|| panic!("invalid address immediate: {src_address}"));
-
-                self.registers.set_single_register(dst, memory_value);
             }
 
             Instruction::LDToRegisterAddress { src, dst } => {
-                let value = self.registers.get_single_register(src);
+                let load_res = self.load_from_register_to_register_address(dst, src);
-                let dst_address = self.registers.get_combined_register(dst);
+                assert_ok!(load_res);
-
-                let memory_value = self.memory.set(dst_address.into(), value);
-
-                assert!(
-                    memory_value.is_some(),
-                    "invalid address stored in ({src}): {dst_address:X}"
-                );
             }
 
             Instruction::LDToImmediateAddress { src, dst_address } => {
-                let register_value = self.registers.get_single_register(src);
+                let load_res = self.load_from_register_to_address(dst_address.into(), src);
-                let memory_value = self.memory.set(dst_address.into(), register_value);
+                assert_ok!(load_res);
-
-                assert!(
-                    memory_value.is_some(),
-                    "invalid address immediate: {dst_address:X}"
-                );
             }
 
             Instruction::LDnToHLAddress { value } => {
-                let dst_address = self.registers.get_combined_register(register::Combined::HL);
+                let dest_address = self.registers.get_combined_register(register::Combined::HL);
-
+                let load_res = self.load_8bit_immediate_to_address(dest_address.into(), value);
-                let memory_value = self.memory.set(dst_address.into(), value);
+                assert_ok!(load_res);
-
-                assert!(
-                    memory_value.is_some(),
-                    "invalid address stored in (hl): {dst_address:X}"
-                );
             }
 
             Instruction::LDFromMemoryRelativeToIORegisterStart {
@@ -86,11 +80,8 @@ impl Processor {
                 let src_address_offset = self.registers.get_single_register(offset_register);
                 let src_address =
                     memory::IO_REGISTER_START_ADDRESS + usize::from(src_address_offset);
-                let memory_value = self.memory.get(src_address).unwrap_or_else(|| {
+                let load_res = self.load_from_address_to_register(dst, src_address);
-                    panic!("invalid offset stored in src register ({offset_register})")
+                assert_ok!(load_res);
-                });
-
-                self.registers.set_single_register(dst, memory_value);
             }
             Instruction::LDToMemoryRelativeToIORegisterStart {
                 src,
@@ -99,37 +90,20 @@ impl Processor {
                 let dst_address_offset = self.registers.get_single_register(offset_register);
                 let dst_address =
                     memory::IO_REGISTER_START_ADDRESS + usize::from(dst_address_offset);
-                let value = self.registers.get_single_register(src);
+                let load_res = self.load_from_register_to_address(dst_address, src);
-                let memory_value = self.memory.set(dst_address, value);
+                assert_ok!(load_res);
-
-                assert!(
-                    memory_value.is_some(),
-                    "invalid offset stored in dst register ({offset_register})"
-                );
             }
             Instruction::LDToRegisterAddressThenDec { dst, src } => {
-                let value = self.registers.get_single_register(src);
                 let dst_address = self.registers.get_combined_register(dst);
-
+                let load_res = self.load_from_register_to_address(dst_address.into(), src);
-                let memory_value = self.memory.set(dst_address.into(), value);
+                assert_ok!(load_res);
-
-                assert!(
-                    memory_value.is_some(),
-                    "invalid address stored in ({src}): {dst_address:X}"
-                );
 
                 self.registers.set_combined_register(dst, dst_address - 1);
             }
             Instruction::LDToRegisterAddressThenInc { dst, src } => {
-                let value = self.registers.get_single_register(src);
                 let dst_address = self.registers.get_combined_register(dst);
-
+                let load_res = self.load_from_register_to_address(dst_address.into(), src);
-                let memory_value = self.memory.set(dst_address.into(), value);
+                assert_ok!(load_res);
-
-                assert!(
-                    memory_value.is_some(),
-                    "invalid address stored in ({src}): {dst_address:X}"
-                );
 
                 self.registers.set_combined_register(dst, dst_address + 1);
             }
@@ -137,6 +111,88 @@ impl Processor {
 
         self.num_cycles += u64::from(instruction.cycles);
     }
+
+    fn load_from_register_to_register_address(
+        &mut self,
+        register_with_dst_address: register::Combined,
+        src: register::Single,
+    ) -> Result<(), Error> {
+        let dst_address = self
+            .registers
+            .get_combined_register(register_with_dst_address);
+
+        let load_res = self.load_from_register_to_address(dst_address.into(), src);
+        // We didn't truly load to an immediate address - we're just using that method for convenience.
+        // map the error as such.
+        if let Err(Error::InvalidImmediateAddress(addr)) = load_res {
+            Err(Error::InvalidRegisterAddress(
+                register_with_dst_address,
+                addr,
+            ))
+        } else {
+            load_res
+        }
+    }
+
+    fn load_from_register_to_address(
+        &mut self,
+        dst_address: usize,
+        src: register::Single,
+    ) -> Result<(), Error> {
+        let register_value = self.registers.get_single_register(src);
+
+        self.load_8bit_immediate_to_address(dst_address, register_value)
+    }
+
+    fn load_8bit_immediate_to_address(
+        &mut self,
+        dst_address: usize,
+        value: u8,
+    ) -> Result<(), Error> {
+        self.memory
+            .set(dst_address, value)
+            .map(|_| ())
+            .ok_or(Error::InvalidImmediateAddress(dst_address))
+    }
+
+    fn load_from_address_to_register(
+        &mut self,
+        dst: register::Single,
+        src_address: usize,
+    ) -> Result<(), Error> {
+        let memory_value = self.memory.get(src_address);
+
+        match memory_value {
+            None => Err(Error::InvalidImmediateAddress(src_address)),
+            Some(value) => {
+                // technically this could be done with map but I think this is more readable
+                self.registers.set_single_register(dst, value);
+                Ok(())
+            }
+        }
+    }
+
+    fn load_from_register_address_to_register(
+        &mut self,
+        dst: register::Single,
+        register_with_src_address: register::Combined,
+    ) -> Result<(), Error> {
+        let src_address = self
+            .registers
+            .get_combined_register(register_with_src_address);
+
+        let load_res = self.load_from_address_to_register(dst, src_address.into());
+        // We didn't truly load to an immediate address - we're just using that method for convenience.
+        // map the error as such.
+        if let Err(Error::InvalidImmediateAddress(addr)) = load_res {
+            Err(Error::InvalidRegisterAddress(
+                register_with_src_address,
+                addr,
+            ))
+        } else {
+            load_res
+        }
+    }
 }
 
 #[cfg(test)]