From e688411cb9f37ac55ce556592656e85c72ee0f91 Mon Sep 17 00:00:00 2001
From: Adrian Conlon <Adrian.conlon@gmail.com>
Date: Sun, 4 Nov 2018 16:38:57 +0000
Subject: [PATCH] Refactor memory related code a little to make the hierarchy
 of classes a little easier to understand.

Signed-off-by: Adrian Conlon <Adrian.conlon@gmail.com>
---
 Intel8080/src/Disassembler.cpp |  2 -
 Intel8080/src/stdafx.h         |  1 -
 LR35902/inc/GameBoyBus.h       |  1 +
 LR35902/src/Disassembler.cpp   |  2 -
 LR35902/src/stdafx.h           |  2 +-
 MC6809/test/Board.h            |  1 +
 MC6809/test/stdafx.h           |  1 +
 Z80/src/Disassembler.cpp       |  2 -
 Z80/src/stdafx.h               |  1 -
 inc/Memory.h                   | 70 ++++++++++++----------------------
 inc/MemoryInterface.h          | 26 -------------
 inc/MemoryMapping.h            |  4 +-
 inc/Ram.h                      | 11 ++++--
 inc/Rom.h                      | 57 +++++++++++++++++++++++++++
 inc/UnusedMemory.h             | 11 +++---
 src/EightBit.vcxproj           |  4 +-
 src/EightBit.vcxproj.filters   | 16 ++++----
 src/{Memory.cpp => Rom.cpp}    |  6 +--
 18 files changed, 112 insertions(+), 106 deletions(-)
 delete mode 100644 inc/MemoryInterface.h
 create mode 100644 inc/Rom.h
 rename src/{Memory.cpp => Rom.cpp} (67%)

diff --git a/Intel8080/src/Disassembler.cpp b/Intel8080/src/Disassembler.cpp
index 00426fc..ec94b29 100644
--- a/Intel8080/src/Disassembler.cpp
+++ b/Intel8080/src/Disassembler.cpp
@@ -2,8 +2,6 @@
 #include "Disassembler.h"
 #include "Intel8080.h"
 
-#include <Memory.h>
-
 #include <sstream>
 #include <iomanip>
 #include <bitset>
diff --git a/Intel8080/src/stdafx.h b/Intel8080/src/stdafx.h
index 0449783..0ae5a4f 100644
--- a/Intel8080/src/stdafx.h
+++ b/Intel8080/src/stdafx.h
@@ -15,6 +15,5 @@
 
 #include <boost/format.hpp>
 
-#include <Memory.h>
 #include <IntelProcessor.h>
 #include <InputOutput.h>
diff --git a/LR35902/inc/GameBoyBus.h b/LR35902/inc/GameBoyBus.h
index 89e3672..1a48ca0 100644
--- a/LR35902/inc/GameBoyBus.h
+++ b/LR35902/inc/GameBoyBus.h
@@ -5,6 +5,7 @@
 #include <string>
 
 #include <Ram.h>
+#include <Rom.h>
 #include <Bus.h>
 #include <Register.h>
 #include <UnusedMemory.h>
diff --git a/LR35902/src/Disassembler.cpp b/LR35902/src/Disassembler.cpp
index 9449a13..d8aa5e6 100644
--- a/LR35902/src/Disassembler.cpp
+++ b/LR35902/src/Disassembler.cpp
@@ -5,8 +5,6 @@
 #include <iomanip>
 #include <bitset>
 
-#include <Memory.h>
-
 #include "LR35902.h"
 #include "IoRegisters.h"
 
diff --git a/LR35902/src/stdafx.h b/LR35902/src/stdafx.h
index 5c050c1..e14ae75 100644
--- a/LR35902/src/stdafx.h
+++ b/LR35902/src/stdafx.h
@@ -25,12 +25,12 @@
 
 #include <boost/format.hpp>
 
-#include <Memory.h>
 #include <Processor.h>
 #include <IntelProcessor.h>
 #include <Register.h>
 #include <Signal.h>
 #include <Ram.h>
+#include <Rom.h>
 #include <Bus.h>
 
 #ifdef _MSC_VER
diff --git a/MC6809/test/Board.h b/MC6809/test/Board.h
index 3163877..04ad606 100644
--- a/MC6809/test/Board.h
+++ b/MC6809/test/Board.h
@@ -5,6 +5,7 @@
 #include <string>
 
 #include <Ram.h>
+#include <Rom.h>
 #include <Bus.h>
 #include <mc6809.h>
 #include <Disassembly.h>
diff --git a/MC6809/test/stdafx.h b/MC6809/test/stdafx.h
index 6024a75..1e3ba2b 100644
--- a/MC6809/test/stdafx.h
+++ b/MC6809/test/stdafx.h
@@ -17,6 +17,7 @@
 #endif
 
 #include <Ram.h>
+#include <Rom.h>
 #include <Bus.h>
 #include <mc6809.h>
 #include <TestHarness.h>
diff --git a/Z80/src/Disassembler.cpp b/Z80/src/Disassembler.cpp
index a73fbf5..4187f19 100644
--- a/Z80/src/Disassembler.cpp
+++ b/Z80/src/Disassembler.cpp
@@ -6,8 +6,6 @@
 #include <bitset>
 #include <iostream>
 
-#include <Memory.h>
-
 #include "Z80.h"
 
 EightBit::Disassembler::Disassembler(Bus& bus) noexcept
diff --git a/Z80/src/stdafx.h b/Z80/src/stdafx.h
index a8e08d0..3bc81dd 100644
--- a/Z80/src/stdafx.h
+++ b/Z80/src/stdafx.h
@@ -16,7 +16,6 @@
 
 #include <boost/format.hpp>
 
-#include <Memory.h>
 #include <IntelProcessor.h>
 #include <InputOutput.h>
 #include <Signal.h>
diff --git a/inc/Memory.h b/inc/Memory.h
index eb4a34f..590a89e 100644
--- a/inc/Memory.h
+++ b/inc/Memory.h
@@ -1,56 +1,34 @@
 #pragma once
 
 #include <cstdint>
-#include <vector>
-#include <string>
 #include <fstream>
-
-#include "MemoryInterface.h"
+#include <stdexcept>
+#include <string>
+#include <vector>
 
 namespace EightBit {
-	class Memory : public MemoryInterface {
-	private:
-		std::vector<uint8_t> m_bytes;
+	// Memory is:
+	// *) Definitely has a size
+	// *) Definitely 'peek'able (although you might not like the answer you get!)
+	// *) Probably 'load'able (i.e. able to be externally initialised)
+	// *) At the implementation level, probably 'poke'able (although may not be exposed to users)
+	// *) Possibly 'reference'able (Very likely if you've exposed 'poke')
+	class Memory {
+	public:
+		virtual ~Memory() = default;
+
+		virtual size_t size() const = 0;
+		virtual uint8_t peek(uint16_t address) const = 0;
+
+		virtual uint8_t& reference(uint16_t) {
+			throw new std::logic_error("Reference operation not allowed.");
+		}
+
+		virtual int load(std::ifstream& file, int writeOffset = 0, int readOffset = 0, int limit = -1) = 0;
+		virtual int load(const std::string& path, int writeOffset = 0, int readOffset = 0, int limit = -1) = 0;
+		virtual int load(const std::vector<uint8_t>& bytes, int writeOffset = 0, int readOffset = 0, int limit = -1) = 0;
 
 	protected:
-		const auto& BYTES() const { return m_bytes; }
-		auto& BYTES() { return m_bytes; }
-
-		virtual void poke(const uint16_t address, const uint8_t value) override {
-			BYTES()[address] = value;
-		}
-
-	public:
-		static int load(std::ifstream& file, std::vector<uint8_t>& output, int writeOffset = 0, int readOffset = 0, int limit = -1, int maximumSize = -1);
-		static int load(const std::string& path, std::vector<uint8_t>& output, int writeOffset = 0, int readOffset = 0, int limit = -1, int maximumSize = -1);
-
-		Memory(const size_t size = 0) noexcept
-		: m_bytes(size) {
-		}
-
-		virtual size_t size() const final { return m_bytes.size();  }
-
-		virtual int load(std::ifstream& file, const int writeOffset = 0, const int readOffset = 0, const int limit = -1) final {
-			const auto maximumSize = (int)size() - writeOffset;
-			return load(file, m_bytes, writeOffset, readOffset, limit, maximumSize);
-		}
-
-		virtual int load(const std::string& path, const int writeOffset = 0, const int readOffset = 0, const int limit = -1) final {
-			const auto maximumSize = (int)size() - writeOffset;
-			return load(path, m_bytes, writeOffset, readOffset, limit, maximumSize);
-		}
-
-		virtual int load(const std::vector<uint8_t>& bytes, const int writeOffset = 0, const int readOffset = 0, int limit = -1) final {
-			if (limit < 0)
-				limit = (int)bytes.size() - readOffset;
-			std::copy(bytes.cbegin() + readOffset, bytes.cbegin() + limit, m_bytes.begin() + writeOffset);
-			return limit;
-		}
-
-		virtual uint8_t peek(const uint16_t address) const final {
-			return BYTES()[address];
-		}
+		virtual void poke(uint16_t address, uint8_t value) = 0;
 	};
-
-	typedef Memory Rom;
 }
diff --git a/inc/MemoryInterface.h b/inc/MemoryInterface.h
deleted file mode 100644
index d5a6ab6..0000000
--- a/inc/MemoryInterface.h
+++ /dev/null
@@ -1,26 +0,0 @@
-#pragma once
-
-#include <cstdint>
-#include <fstream>
-#include <stdexcept>
-#include <string>
-#include <vector>
-
-namespace EightBit {
-	class MemoryInterface {
-	public:
-		virtual size_t size() const = 0;
-		virtual uint8_t peek(uint16_t address) const = 0;
-
-		virtual uint8_t& reference(uint16_t) {
-			throw new std::logic_error("Reference operation not allowed.");
-		}
-
-		virtual int load(std::ifstream& file, int writeOffset = 0, int readOffset = 0, int limit = -1) = 0;
-		virtual int load(const std::string& path, int writeOffset = 0, int readOffset = 0, int limit = -1) = 0;
-		virtual int load(const std::vector<uint8_t>& bytes, int writeOffset = 0, int readOffset = 0, int limit = -1) = 0;
-
-	protected:
-		virtual void poke(uint16_t address, uint8_t value) = 0;
-	};
-}
diff --git a/inc/MemoryMapping.h b/inc/MemoryMapping.h
index 71bccdd..35428d3 100644
--- a/inc/MemoryMapping.h
+++ b/inc/MemoryMapping.h
@@ -4,13 +4,13 @@
 
 namespace EightBit {
 
-	class MemoryInterface;
+	class Memory;
 
 	struct MemoryMapping {
 
 		enum AccessLevel { Unknown, ReadOnly, ReadWrite, };
 
-		MemoryInterface& memory;
+		Memory& memory;
 		uint16_t begin = 0xffff;
 		uint16_t mask = 0U;
 		AccessLevel access = Unknown;
diff --git a/inc/Ram.h b/inc/Ram.h
index 69a3818..66214eb 100644
--- a/inc/Ram.h
+++ b/inc/Ram.h
@@ -1,13 +1,16 @@
 #pragma once
 
 #include <cstdint>
-#include "Memory.h"
+
+#include "Rom.h"
 
 namespace EightBit {
-	class Ram : public Memory {
+	// The RAM class is everything the ROM class is, plus
+	// it's externally 'reference'able and 'poke'able.
+	class Ram : public Rom {
 	public:
 		Ram(const size_t size = 0) noexcept
-		: Memory(size) {
+		: Rom(size) {
 		}
 
 		virtual uint8_t& reference(const uint16_t address) final {
@@ -15,7 +18,7 @@ namespace EightBit {
 		}
 
 		virtual void poke(const uint16_t address, const uint8_t value) final {
-			Memory::poke(address, value);
+			Rom::poke(address, value);
 		}
 	};
 }
diff --git a/inc/Rom.h b/inc/Rom.h
new file mode 100644
index 0000000..c27c42e
--- /dev/null
+++ b/inc/Rom.h
@@ -0,0 +1,57 @@
+#pragma once
+
+#include <cstdint>
+#include <vector>
+#include <string>
+#include <fstream>
+
+#include "Memory.h"
+
+namespace EightBit {
+	// ROM is a basic implementation of the Memory interface.
+	// Nothing over and above the interface is exposed to users
+	// of the ROM class.
+	class Rom : public Memory {
+	private:
+		std::vector<uint8_t> m_bytes;
+
+	protected:
+		const auto& BYTES() const { return m_bytes; }
+		auto& BYTES() { return m_bytes; }
+
+		virtual void poke(const uint16_t address, const uint8_t value) override {
+			BYTES()[address] = value;
+		}
+
+	public:
+		static int load(std::ifstream& file, std::vector<uint8_t>& output, int writeOffset = 0, int readOffset = 0, int limit = -1, int maximumSize = -1);
+		static int load(const std::string& path, std::vector<uint8_t>& output, int writeOffset = 0, int readOffset = 0, int limit = -1, int maximumSize = -1);
+
+		Rom(const size_t size = 0) noexcept
+		: m_bytes(size) {
+		}
+
+		virtual size_t size() const final { return m_bytes.size();  }
+
+		virtual int load(std::ifstream& file, const int writeOffset = 0, const int readOffset = 0, const int limit = -1) final {
+			const auto maximumSize = (int)size() - writeOffset;
+			return load(file, m_bytes, writeOffset, readOffset, limit, maximumSize);
+		}
+
+		virtual int load(const std::string& path, const int writeOffset = 0, const int readOffset = 0, const int limit = -1) final {
+			const auto maximumSize = (int)size() - writeOffset;
+			return load(path, m_bytes, writeOffset, readOffset, limit, maximumSize);
+		}
+
+		virtual int load(const std::vector<uint8_t>& bytes, const int writeOffset = 0, const int readOffset = 0, int limit = -1) final {
+			if (limit < 0)
+				limit = (int)bytes.size() - readOffset;
+			std::copy(bytes.cbegin() + readOffset, bytes.cbegin() + limit, m_bytes.begin() + writeOffset);
+			return limit;
+		}
+
+		virtual uint8_t peek(const uint16_t address) const final {
+			return BYTES()[address];
+		}
+	};
+}
diff --git a/inc/UnusedMemory.h b/inc/UnusedMemory.h
index d81c5d6..1824239 100644
--- a/inc/UnusedMemory.h
+++ b/inc/UnusedMemory.h
@@ -1,9 +1,12 @@
 #pragma once
 
-#include "MemoryInterface.h"
+#include "Memory.h"
 
 namespace EightBit {
-	class UnusedMemory final : public MemoryInterface {
+	// A read-only Memory implementation that has a fixed size and will
+	// *always* returns the same value, from whichever location
+	// is being read.
+	class UnusedMemory final : public Memory {
 	public:
 		UnusedMemory(const size_t size, const uint8_t value)
 		: m_size(size), m_value(value) {}
@@ -12,10 +15,6 @@ namespace EightBit {
 		virtual size_t size() const final { return m_size; }
 		virtual uint8_t peek(uint16_t address) const final { return m_value; }
 
-		virtual uint8_t& reference(uint16_t) {
-			throw new std::logic_error("Reference operation not allowed.");
-		}
-
 		virtual int load(std::ifstream& file, int writeOffset = 0, int readOffset = 0, int limit = -1) final {
 			throw new std::logic_error("load operation not allowed.");
 		}
diff --git a/src/EightBit.vcxproj b/src/EightBit.vcxproj
index 23b0523..aa651d2 100644
--- a/src/EightBit.vcxproj
+++ b/src/EightBit.vcxproj
@@ -148,8 +148,8 @@
     <ClInclude Include="..\inc\InputOutput.h" />
     <ClInclude Include="..\inc\IntelProcessor.h" />
     <ClInclude Include="..\inc\LittleEndianProcessor.h" />
+    <ClInclude Include="..\inc\Rom.h" />
     <ClInclude Include="..\inc\Memory.h" />
-    <ClInclude Include="..\inc\MemoryInterface.h" />
     <ClInclude Include="..\inc\MemoryMapping.h" />
     <ClInclude Include="..\inc\Processor.h" />
     <ClInclude Include="..\inc\Ram.h" />
@@ -167,7 +167,7 @@
     <ClCompile Include="InputOutput.cpp" />
     <ClCompile Include="IntelProcessor.cpp" />
     <ClCompile Include="LittleEndianProcessor.cpp" />
-    <ClCompile Include="Memory.cpp" />
+    <ClCompile Include="Rom.cpp" />
     <ClCompile Include="Processor.cpp" />
     <ClCompile Include="stdafx.cpp">
       <PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">Create</PrecompiledHeader>
diff --git a/src/EightBit.vcxproj.filters b/src/EightBit.vcxproj.filters
index a16718d..b110c5d 100644
--- a/src/EightBit.vcxproj.filters
+++ b/src/EightBit.vcxproj.filters
@@ -17,9 +17,6 @@
     <ClInclude Include="..\inc\EventArgs.h">
       <Filter>Header Files</Filter>
     </ClInclude>
-    <ClInclude Include="..\inc\Memory.h">
-      <Filter>Header Files</Filter>
-    </ClInclude>
     <ClInclude Include="..\inc\Processor.h">
       <Filter>Header Files</Filter>
     </ClInclude>
@@ -59,10 +56,13 @@
     <ClInclude Include="..\inc\Chip.h">
       <Filter>Header Files</Filter>
     </ClInclude>
-    <ClInclude Include="..\inc\MemoryInterface.h">
+    <ClInclude Include="..\inc\UnusedMemory.h">
       <Filter>Header Files</Filter>
     </ClInclude>
-    <ClInclude Include="..\inc\UnusedMemory.h">
+    <ClInclude Include="..\inc\Rom.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>
+    <ClInclude Include="..\inc\Memory.h">
       <Filter>Header Files</Filter>
     </ClInclude>
   </ItemGroup>
@@ -73,9 +73,6 @@
     <ClCompile Include="EventArgs.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
-    <ClCompile Include="Memory.cpp">
-      <Filter>Source Files</Filter>
-    </ClCompile>
     <ClCompile Include="Processor.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
@@ -97,5 +94,8 @@
     <ClCompile Include="Chip.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="Rom.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
   </ItemGroup>
 </Project>
\ No newline at end of file
diff --git a/src/Memory.cpp b/src/Rom.cpp
similarity index 67%
rename from src/Memory.cpp
rename to src/Rom.cpp
index d66968a..5eb3da7 100644
--- a/src/Memory.cpp
+++ b/src/Rom.cpp
@@ -1,9 +1,9 @@
 #include "stdafx.h"
-#include "Memory.h"
+#include "Rom.h"
 
 #include <iostream>
 
-int EightBit::Memory::load(std::ifstream& file, std::vector<uint8_t>& output, const int writeOffset, const int readOffset, int limit, const int maximumSize) {
+int EightBit::Rom::load(std::ifstream& file, std::vector<uint8_t>& output, const int writeOffset, const int readOffset, int limit, const int maximumSize) {
 
 	file.seekg(0, std::ios::end);
 
@@ -25,7 +25,7 @@ int EightBit::Memory::load(std::ifstream& file, std::vector<uint8_t>& output, co
 	return size;
 }
 
-int EightBit::Memory::load(const std::string& path, std::vector<uint8_t>& output, const int writeOffset, const int readOffset, const int limit, const int maximumSize) {
+int EightBit::Rom::load(const std::string& path, std::vector<uint8_t>& output, const int writeOffset, const int readOffset, const int limit, const int maximumSize) {
 
 	std::ifstream file;
 	file.exceptions(std::ios::failbit | std::ios::badbit);