Comments on ByteWatch

Also moved nullable value type to non nullable (with exception
prevention).

BizHawk.Client.Common/tools/Watch/ByteWatch.cs

@@ -1,13 +1,17 @@
-using BizHawk.Common.NumberExtensions;
+using System;
-using BizHawk.Common.StringExtensions;
-using BizHawk.Emulation.Common;
-using System;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
 
+using BizHawk.Common.NumberExtensions;
+using BizHawk.Common.StringExtensions;
+using BizHawk.Emulation.Common;
+
 namespace BizHawk.Client.Common
 {
+	/// <summary>
+	/// This class holds a byte (8 bits) <see cref="Watch"/>
+	/// </summary>
 	public sealed class ByteWatch : Watch
 	{
 		#region Fields
@@ -19,6 +23,18 @@ namespace BizHawk.Client.Common
 
 		#region cTor(s)
 
+		/// <summary>
+		/// Inialize a new instance of <see cref="ByteWatch"/>
+		/// </summary>
+		/// <param name="domain"><see cref="MemoryDomain"/> where you want to track</param>
+		/// <param name="address">The address you want to track</param>
+		/// <param name="type">How you you want to display the value See <see cref="DisplayType"/></param>
+		/// <param name="bigEndian">Specify the endianess. true for big endian</param>
+		/// <param name="note">A custom note about the <see cref="Watch"/></param>
+		/// <param name="value">Current value</param>
+		/// <param name="previous">Previous value</param>
+		/// <param name="changeCount">How many times value has changed</param>
+		/// <exception cref="ArgumentException">Occurs when a <see cref="DisplayType"/> is incompatible with <see cref="WatchSize.Byte"/></exception>
 		internal ByteWatch(MemoryDomain domain, long address, DisplayType type, bool bigEndian, string note, byte value, byte previous, int changeCount)
 			: base(domain, address, WatchSize.Byte, type, bigEndian, note)
 		{
@@ -27,15 +43,13 @@ namespace BizHawk.Client.Common
 			this._changecount = changeCount;
 		}
 
-		internal ByteWatch(MemoryDomain domain, long address, DisplayType type, bool bigEndian, string note)
-			:this(domain, address, type, bigEndian, note, 0, 0, 0)
-		{
-			_previous = GetByte();
-			_value = GetByte();			
-		}		
-
 		#endregion
 
+		#region Methods		
+
+		/// <summary>
+		/// Enumerate wich <see cref="DisplayType"/> are valid for a <see cref="ByteWatch"/>
+		/// </summary>
 		public static IEnumerable<DisplayType> ValidTypes
 		{
 			get
@@ -47,54 +61,59 @@ namespace BizHawk.Client.Common
 			}
 		}
 
+		#region Implements
+
+		/// <summary>
+		/// Get a list a <see cref="DisplayType"/> that can be used for this <see cref="ByteWatch"/>
+		/// </summary>
+		/// <returns>An enumartion that contains all valid <see cref="DisplayType"/></returns>
 		public override IEnumerable<DisplayType> AvailableTypes()
 		{
-			yield return DisplayType.Unsigned;
+			return ValidTypes;
-			yield return DisplayType.Signed;
-			yield return DisplayType.Hex;
-			yield return DisplayType.Binary;
-        }
-
-		public override int? Value
-		{
-			get { return GetByte(); }
-		}
-
-		public override int? ValueNoFreeze
-		{
-			get { return GetByte(true); }
-		}
-
-		public override string ValueString
-		{
-			get { return FormatValue(GetByte()); }
-		}
-
-		public override int? Previous
-		{
-			get { return _previous; }
-		}
-
-		public override string PreviousStr
-		{
-			get { return FormatValue(_previous); }
 		}
 
+		/// <summary>
+		/// Reset the previous value; set it to the current one
+		/// </summary>
 		public override void ResetPrevious()
 		{
 			_previous = GetByte();
 		}
 
-		public override string ToString()
+		/// <summary>
+		/// Update the Watch (read it from <see cref="MemoryDomain"/>
+		/// </summary>
+		public override void Update()
 		{
-			return Notes + ": " + ValueString;
+			switch (Global.Config.RamWatchDefinePrevious)
+			{
+				case PreviousType.Original:
+					return;
+				case PreviousType.LastChange:
+					var temp = _value;
+					_value = GetByte();
+					if (_value != temp)
+					{
+						_previous = _value;
+						_changecount++;
 					}
 
-		public override uint MaxValue
+					break;
+				case PreviousType.LastFrame:
+					_previous = _value;
+					_value = GetByte();
+					if (_value != Previous)
 					{
-			get { return byte.MaxValue; }
+						_changecount++;
 					}
 
+					break;
+			}
+		}
+
+		#endregion
+
+		//TODO: Implements IFormattable
 		public string FormatValue(byte val)
 		{
 			switch (Type)
@@ -111,6 +130,92 @@ namespace BizHawk.Client.Common
 			}
 		}
 
+		#endregion
+
+		public override string ToString()
+		{			
+			return Notes + ": " + ValueString;
+		}
+
+		#region Properties
+
+		#region Implements
+
+		/// <summary>
+		/// Get a string representation of difference
+		/// between current value and the previous one
+		/// </summary>
+		public override string Diff
+		{
+			get
+			{
+				string diff = string.Empty;
+				byte diffVal = Convert.ToByte(_value - _previous);
+				if (diffVal > 0)
+				{
+					diff = "+";
+				}
+				else if (diffVal < 0)
+				{
+					diff = "-";
+				}
+
+				return string.Format("{0}{1}", diff, FormatValue(diffVal));
+			}
+		}
+
+		/// <summary>
+		/// Get the maximum possible value
+		/// </summary>
+		public override uint MaxValue
+		{
+			get
+			{
+				return byte.MaxValue;
+			}
+		}
+
+		/// <summary>
+		/// Get the current value
+		/// </summary>
+		public override int Value
+		{
+			get
+			{
+				return GetByte();
+			}
+		}
+
+		/// <summary>
+		/// Gets the current value
+		/// but with stuff I don't understand
+		/// </summary>
+		/// <remarks>zero 15-nov-2015 - bypass LIAR LOGIC, see fdc9ea2aa922876d20ba897fb76909bf75fa6c92 https://github.com/TASVideos/BizHawk/issues/326 </remarks>
+		public override int ValueNoFreeze
+		{
+			get
+			{
+				return GetByte(true);
+			}
+		}
+
+		/// <summary>
+		/// Get a string representation of the current value
+		/// </summary>
+		public override string ValueString
+		{
+			get
+			{
+				return FormatValue(_value);
+			}
+		}
+
+		/// <summary>
+		/// Try to sets the value into the <see cref="MemoryDomain"/>
+		/// at the current <see cref="Watch"/> address
+		/// </summary>
+		/// <param name="value">Value to set</param>
+		/// <returns>True if value successfully sets; othewise, false</returns>
 		public override bool Poke(string value)
 		{
 			try
@@ -185,51 +290,30 @@ namespace BizHawk.Client.Common
 			}
 		}
 
-		public override string Diff
+		/// <summary>
+		/// Get the previous value
+		/// </summary>
+		public override int Previous
 		{
 			get
 			{
-				var diff = string.Empty;
+				return _previous;
-				var diffVal = _value - _previous;
-				if (diffVal > 0)
-				{
-					diff = "+";
-				}
-				else if (diffVal < 0)
-				{
-					diff = "-";
-				}
-
-				return diff + FormatValue((byte)(_previous - _value));
 			}
 		}
 
-		public override void Update()
+		/// <summary>
+		/// Get a string representation of the previous value
+		/// </summary>
+		public override string PreviousStr
 		{
-			switch (Global.Config.RamWatchDefinePrevious)
+			get
 			{
-				case PreviousType.Original:
+				return FormatValue(_previous);
-					return;
+			}
-				case PreviousType.LastChange:
-					var temp = _value;
-					_value = GetByte();
-					if (_value != temp)
-					{
-						_previous = _value;
-						_changecount++;
 		}
 
-					break;
+		#endregion Implements
-				case PreviousType.LastFrame:
-					_previous = _value;
-					_value = GetByte();
-					if (_value != Previous)
-					{
-						_changecount++;
-					}
 
-					break;
+		#endregion		
-			}
-		}
 	}
 }

BizHawk.Client.Common/tools/Watch/DwordWatch.cs

@@ -62,17 +62,17 @@ namespace BizHawk.Client.Common
 			yield return DisplayType.Float;
         }
 
-		public override int? Value
+		public override int Value
 		{
 			get { return (int)GetDWord(); }
 		}
 
-		public override int? ValueNoFreeze
+		public override int ValueNoFreeze
 		{
 			get { return (int)GetDWord(true); }
 		}
 
-		public override int? Previous
+		public override int Previous
 		{
 			get { return (int)_previous; }
 		}

BizHawk.Client.Common/tools/Watch/SeparatorWatch.cs

@@ -14,19 +14,19 @@ namespace BizHawk.Client.Common
 			get { return new SeparatorWatch(); }
 		}
 
-		public override int? Value
+		public override int Value
 		{
-			get { return null; }
+			get { return 0; }
 		}
 
-		public override int? ValueNoFreeze
+		public override int ValueNoFreeze
 		{
-			get { return null; }
+			get { return 0; }
 		}
 
-		public override int? Previous
+		public override int Previous
 		{
-			get { return null; }
+			get { return 0; }
 		}		
 
 		public override string ValueString

BizHawk.Client.Common/tools/Watch/Watch.cs

@@ -1,13 +1,18 @@
-using BizHawk.Common.NumberExtensions;
+using System;
-using BizHawk.Emulation.Common;
-
-using System;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
 
+using BizHawk.Common.NumberExtensions;
+using BizHawk.Emulation.Common;
+
 namespace BizHawk.Client.Common
 {
+	/// <summary>
+	/// This class holds a watch i.e. something inside a <see cref="MemoryDomain"/> identified by an address
+	/// with a specific size (8, 16 or 32bits).
+	/// This is an abstract class
+	/// </summary>
 	public abstract partial class Watch
 		: IEquatable<Watch>,
 		IEquatable<Cheat>,
@@ -311,7 +316,7 @@ namespace BizHawk.Client.Common
 		public abstract IEnumerable<DisplayType> AvailableTypes();
 
 		/// <summary>
-		/// Reset the previous value
+		/// Reset the previous value; set it to the current one
 		/// </summary>
 		public abstract void ResetPrevious();
 
@@ -329,7 +334,7 @@ namespace BizHawk.Client.Common
 			if (!bypassFreeze && Global.CheatList.IsActive(_domain, _address))
 			{
 				//LIAR logic				
-				return Global.CheatList.GetByteValue(_domain, _address).Value;
+				return Global.CheatList.GetByteValue(_domain, _address) ?? 0;
 			}
 			else
 			{
@@ -349,7 +354,7 @@ namespace BizHawk.Client.Common
 			if (!bypassFreeze && Global.CheatList.IsActive(_domain, _address))
 			{
 				//LIAR logic
-				return (ushort)Global.CheatList.GetCheatValue(_domain, _address, WatchSize.Word).Value;
+				return (ushort)(Global.CheatList.GetCheatValue(_domain, _address, WatchSize.Word) ?? 0);
 			}
 			else
 			{
@@ -369,7 +374,7 @@ namespace BizHawk.Client.Common
 			if (!bypassFreeze && Global.CheatList.IsActive(_domain, _address))
 			{
 				//LIAR logic
-				return (uint)Global.CheatList.GetCheatValue(_domain, _address, WatchSize.DWord).Value;
+				return (uint)(Global.CheatList.GetCheatValue(_domain, _address, WatchSize.DWord) ?? 0);
 			}
 			else
 			{
@@ -424,7 +429,8 @@ namespace BizHawk.Client.Common
 		/// <returns>True if both object are equals; otherwise, false</returns>
 		public bool Equals(Watch other)
 		{
-			return this._domain == other._domain &&
+			return !object.ReferenceEquals(other, null) &&
+					this._domain == other._domain &&
 					this._address == other._address &&
 					this._size == other._size;
 		}
@@ -440,7 +446,8 @@ namespace BizHawk.Client.Common
 		/// <returns>True if both object are equals; otherwise, false</returns>
 		public bool Equals(Cheat other)
 		{
-			return this._domain == other.Domain &&
+			return !object.ReferenceEquals(other, null) &&
+					this._domain == other.Domain &&
 					this._address == other.Address &&
 					this._size == other.Size;
 		}
@@ -467,6 +474,10 @@ namespace BizHawk.Client.Common
 			{
 				return 0;
 			}
+			else if (object.ReferenceEquals(other, null))
+			{
+				return 1;
+			}
             else if (_address.Equals(other._address))
 			{
 				return ((int)_size).CompareTo((int)other._size);
@@ -543,14 +554,50 @@ namespace BizHawk.Client.Common
 
 		#region Abstracts
 
+		/// <summary>
+		/// Get a string representation of difference
+		/// between current value and the previous one
+		/// </summary>
 		public abstract string Diff { get; }
+
+		/// <summary>
+		/// Get the maximum possible value
+		/// </summary>
 		public abstract uint MaxValue { get; }
-		public abstract int? Value { get; }
+
-		//zero 15-nov-2015 - bypass LIAR LOGIC, see fdc9ea2aa922876d20ba897fb76909bf75fa6c92 https://github.com/TASVideos/BizHawk/issues/326
+		/// <summary>
-		public abstract int? ValueNoFreeze { get; }
+		/// Get the current value
+		/// </summary>
+		public abstract int Value { get; }
+
+		/// <summary>
+		/// Gets the current value
+		/// but with stuff I don't understand
+		/// </summary>
+		/// <remarks>zero 15-nov-2015 - bypass LIAR LOGIC, see fdc9ea2aa922876d20ba897fb76909bf75fa6c92 https://github.com/TASVideos/BizHawk/issues/326 </remarks>
+		public abstract int ValueNoFreeze { get; }
+
+		/// <summary>
+		/// Get a string representation of the current value
+		/// </summary>
 		public abstract string ValueString { get; }
+
+		/// <summary>
+		/// Try to sets the value into the <see cref="MemoryDomain"/>
+		/// at the current <see cref="Watch"/> address
+		/// </summary>
+		/// <param name="value">Value to set</param>
+		/// <returns>True if value successfully sets; othewise, false</returns>
 		public abstract bool Poke(string value);
-		public abstract int? Previous { get; }
+
+		/// <summary>
+		/// Get the previous value
+		/// </summary>
+		public abstract int Previous { get; }
+
+		/// <summary>
+		/// Get a string representation of the previous value
+		/// </summary>
 		public abstract string PreviousStr { get; }
 
 		#endregion Abstracts
@@ -728,6 +775,7 @@ namespace BizHawk.Client.Common
 
 		#endregion
 
+		//TODO: Replace all the following stuff by implementing ISerializable
 		public static string DisplayTypeToString(DisplayType type)
 		{
 			switch (type)

BizHawk.Client.Common/tools/Watch/WatchList.cs

@@ -118,7 +118,7 @@ namespace BizHawk.Client.Common
 					if (reverse)
 					{
 						_watchList = _watchList
-							.OrderByDescending(x => x.Address ?? 0)
+							.OrderByDescending(x => x.Address)
 							.ThenBy(x => x.Domain.Name)
 							.ThenBy(x => x.Size)
 							.ThenBy(x => x.Type)
@@ -128,7 +128,7 @@ namespace BizHawk.Client.Common
 					else
 					{
 						_watchList = _watchList
-							.OrderBy(x => x.Address ?? 0)
+							.OrderBy(x => x.Address)
 							.ThenBy(x => x.Domain.Name)
 							.ThenBy(x => x.Size)
 							.ThenBy(x => x.Type)
@@ -141,8 +141,8 @@ namespace BizHawk.Client.Common
 					if (reverse)
 					{
 						_watchList = _watchList
-							.OrderByDescending(x => x.Value ?? 0)
+							.OrderByDescending(x => x.Value)
-							.ThenBy(x => x.Address ?? 0)
+							.ThenBy(x => x.Address)
 							.ThenBy(x => x.Size)
 							.ThenBy(x => x.Type)
 							.ThenBy(x => x.BigEndian)
@@ -151,8 +151,8 @@ namespace BizHawk.Client.Common
 					else
 					{
 						_watchList = _watchList
-							.OrderBy(x => x.Value ?? 0)
+							.OrderBy(x => x.Value)
-							.ThenBy(x => x.Address ?? 0)
+							.ThenBy(x => x.Address)
 							.ThenBy(x => x.Size)
 							.ThenBy(x => x.Type)
 							.ThenBy(x => x.BigEndian)

BizHawk.Client.Common/tools/Watch/WordWatch.cs

@@ -62,17 +62,17 @@ namespace BizHawk.Client.Common
 			get { return ushort.MaxValue; }
 		}
 
-		public override int? Value
+		public override int Value
 		{
 			get { return GetWord(); }
 		}
 
-		public override int? ValueNoFreeze
+		public override int ValueNoFreeze
 		{
 			get { return GetWord(true); }
 		}
 
-		public override int? Previous
+		public override int Previous
 		{
 			get { return _previous; }
 		}

BizHawk.Client.EmuHawk/Extensions/ToolExtensions.cs

@@ -234,7 +234,7 @@ namespace BizHawk.Client.EmuHawk.ToolExtensions
 			Global.CheatList.AddRange(
 				watches
 				.Where(w => !w.IsSeparator)
-				.Select(w => new Cheat(w, w.Value ?? 0)));
+				.Select(w => new Cheat(w, w.Value)));
 		}
 
 		public static void UnfreezeAll(this IEnumerable<Watch> watches)

BizHawk.Client.EmuHawk/tools/HexEditor/HexEditor.cs

@@ -762,7 +762,7 @@ namespace BizHawk.Client.EmuHawk
 
 				Global.CheatList.Add(new Cheat(
 					watch,
-					watch.Value ?? 0));
+					watch.Value));
 			}
 		}
 
@@ -780,7 +780,7 @@ namespace BizHawk.Client.EmuHawk
 
 				cheats.Add(new Cheat(
 					watch,
-					watch.Value ?? 0));
+					watch.Value));
 			}
 
 			Global.CheatList.AddRange(cheats);