Slight refactor, plus cleanup

SA_DiceExpression.h

@@ -99,28 +99,56 @@ NSComparisonResult compareEvaluatedExpressionsByAttemptBonus(SA_DiceExpression *
 #pragma mark - Properties
 /************************/
 
+// The expression’s type (operation, roll command, simple value, etc.).
 @property SA_DiceExpressionTermType type;
 
-@property NSUInteger errorBitMask;
+/*==============================================================================
+ The following four sets of properties pertain to expressions of specific types.
+ */
 
+// Expressions of type SA_DiceExpressionTerm_OPERATION.
 @property SA_DiceExpressionOperator operator;
 @property (nonatomic, strong) SA_DiceExpression *leftOperand;
 @property (nonatomic, strong) SA_DiceExpression *rightOperand;
 
+// Expressions of type SA_DiceExpressionTerm_ROLL_COMMAND.
 @property SA_DiceExpressionRollCommand rollCommand;
 @property (nonatomic, strong) SA_DiceExpression *dieCount;
 @property (nonatomic, strong) SA_DiceExpression *dieSize;
 @property SA_DiceExpressionDieType dieType;
 
+// Expressions of type SA_DiceExpressionTerm_ROLL_MODIFIER.
 @property SA_DiceExpressionRollModifier rollModifier;
 
+// Expressions of type SA_DiceExpressionTerm_VALUE.
 @property (nonatomic, strong) NSNumber *value;
 
+/*===================================================
+ The following properties pertain to all expressions.
+ */
+
+@property SA_DiceExpressionError errorBitMask;
+
 @property (copy, nonatomic) NSString *inputString;
 @property (copy, nonatomic) NSAttributedString *attributedInputString;
 
+/*=========================================================================
+ The following properties pertain to evaluated expressions only.
+ (They have a nil value for expressions which have not yet been evaluated.)
+ */
+
+// Evaluated expressions (of any type).
 @property (nonatomic, strong) NSNumber *result;
 
+// Evaluated expressions of type SA_DiceExpressionTerm_ROLL_COMMAND.
 @property (nonatomic, strong) NSArray <NSNumber *> *rolls;
 
+/****************************/
+#pragma mark - Public methods
+/****************************/
+
++(instancetype) expressionByJoiningExpression:(SA_DiceExpression *)leftHandExpression
+								 toExpression:(SA_DiceExpression *)rightHandExpression
+								 withOperator:(SA_DiceExpressionOperator)operator;
+
 @end

SA_DiceExpression.m

@@ -8,6 +8,8 @@
 
 #import "SA_DiceExpression.h"
 
+#import "SA_DiceFormatter.h"
+
 /*********************/
 #pragma mark Functions
 /*********************/
@@ -74,11 +76,19 @@ NSString *NSStringFromSA_DiceExpressionError(SA_DiceExpressionError error) {
 												@(SA_DiceExpressionError_INTEGER_UNDERFLOW_MULTIPLICATION)		: @"SA_DB_ERROR_INTEGER_UNDERFLOW_MULTIPLICATION",
 												@(SA_DiceExpressionError_KEEP_COUNT_EXCEEDS_ROLL_COUNT)			: @"SA_DB_ERROR_KEEP_COUNT_EXCEEDS_ROLL_COUNT",
 												@(SA_DiceExpressionError_KEEP_COUNT_NEGATIVE)					: @"SA_DB_ERROR_KEEP_COUNT_NEGATIVE"
-
 												};
 	});
 
-	return SA_DiceExpressionErrorStringValues[@(error)];
+	__block NSMutableArray *errorStrings = [NSMutableArray array];
+	[SA_DiceExpressionErrorStringValues enumerateKeysAndObjectsUsingBlock:^(NSNumber *errorKey,
+																			NSString *errorString,
+																			BOOL *stop) {
+		if (errorKey.unsignedIntegerValue & error)
+			[errorStrings addObject:errorString];
+	}];
+	return [errorStrings componentsJoinedByString:@","];
+
+//	return SA_DiceExpressionErrorStringValues[@(error)];
 }
 
 NSComparisonResult compareEvaluatedExpressionsByResult(SA_DiceExpression *expression1,
@@ -109,6 +119,65 @@ NSComparisonResult compareEvaluatedExpressionsByAttemptBonus(SA_DiceExpression *
 
 @implementation SA_DiceExpression
 
++(SA_DiceExpression *) expressionByJoiningExpression:(SA_DiceExpression *)leftHandExpression
+										toExpression:(SA_DiceExpression *)rightHandExpression
+										withOperator:(SA_DiceExpressionOperator)operator {
+	SA_DiceExpression *expression = [SA_DiceExpression new];
+
+	// First, we check that the operands and operator are not nil. If they are,
+	// then the expression is invalid...
+	if (   leftHandExpression == nil
+		|| rightHandExpression == nil
+		|| operator == SA_DiceExpressionOperator_NONE
+		) {
+		expression.type = SA_DiceExpressionTerm_NONE;
+		expression.errorBitMask |= SA_DiceExpressionError_INVALID_EXPRESSION;
+		return expression;
+	}
+
+	// If the operands and operator are present, then the expression is an
+	// operation expression...
+	expression.type = SA_DiceExpressionTerm_OPERATION;
+
+	// ... but does it have a valid operator?
+	if (   operator == SA_DiceExpressionOperator_MINUS
+		|| operator == SA_DiceExpressionOperator_PLUS
+		|| operator == SA_DiceExpressionOperator_TIMES
+		) {
+		expression.operator = operator;
+	} else {
+		expression.errorBitMask |= SA_DiceExpressionError_UNKNOWN_OPERATOR;
+		return expression;
+	}
+
+	// The operator is valid. Set the operands...
+	expression.leftOperand = leftHandExpression;
+	expression.rightOperand = rightHandExpression;
+
+	// And inherit any errors that they may have.
+	expression.errorBitMask |= expression.leftOperand.errorBitMask;
+	expression.errorBitMask |= expression.rightOperand.errorBitMask;
+
+	// Since this top-level expression was NOT generated by parsing an input
+	// string, for completeness and consistency, we have to generate a fake
+	// input string ourselves! We do this by wrapping each operand in
+	// parentheses and putting the canonical representation of the operator
+	// between them.
+	// TODO: Shouldn't the canonical representation of an operator
+	// possibly vary by formatter behavior...?
+	// But on the other hand, how does a parser know what formatter behavior
+	// is in use...?
+	// TODO: Parentheses aren't even supported by the legacy parser!
+	// This is total nonsense!
+	expression.inputString = [NSString stringWithFormat:@"(%@)%@(%@)",
+							  expression.leftOperand.inputString,
+							  [SA_DiceFormatter canonicalRepresentationForOperator:expression.operator],
+							  expression.rightOperand.inputString];
+
+	// The joining is complete. (Power overwhelming.)
+	return expression;
+}
+
 /*******************************/
 #pragma mark - NSCopying methods
 /*******************************/

SA_DiceParser.h

@@ -96,6 +96,8 @@
  
  NOTE 2: Legacy mode does not support whitespace within roll strings.
  
+ TODO: Document exploding dice, Fudge dice, and ‘k’ and ‘l’ operators.
+ 
  =====================
  ==== MODERN mode ====
  =====================
@@ -152,8 +154,4 @@ typedef NS_ENUM(NSUInteger, SA_DiceParserBehavior) {
 
 -(SA_DiceExpression *) expressionForString:(NSString *)dieRollString;
 
--(SA_DiceExpression *) expressionByJoiningExpression:(SA_DiceExpression *)leftHandExpression
-										toExpression:(SA_DiceExpression *)rightHandExpression
-										withOperator:(SA_DiceExpressionOperator)operator;
-
 @end

SA_DiceParser.m

@@ -68,6 +68,7 @@ static NSDictionary *_validCharactersDict;
 	return _defaultParserBehavior;
 }
 
+// TODO: Should this be on a per-mode, and therefore per-instance, basis?
 +(NSDictionary *) validCharactersDict {
 	if (_validCharactersDict == nil) {
 		[SA_DiceParser loadValidCharactersDict];
@@ -85,13 +86,15 @@ static NSDictionary *_validCharactersDict;
 }
 
 -(instancetype) initWithBehavior:(SA_DiceParserBehavior)parserBehavior {
-	if (self = [super init]) {
-		self.parserBehavior = parserBehavior;
-		
-		if (_validCharactersDict == nil) {
-			[SA_DiceParser loadValidCharactersDict];
-		}
+	if (!(self = [super init]))
+		return nil;
+
+	self.parserBehavior = parserBehavior;
+
+	if (_validCharactersDict == nil) {
+		[SA_DiceParser loadValidCharactersDict];
 	}
+
 	return self;
 }
 
@@ -115,57 +118,6 @@ static NSDictionary *_validCharactersDict;
 	}
 }
 
--(SA_DiceExpression *) expressionByJoiningExpression:(SA_DiceExpression *)leftHandExpression
-										toExpression:(SA_DiceExpression *)rightHandExpression
-										withOperator:(SA_DiceExpressionOperator)operator {
-	SA_DiceExpression *expression = [SA_DiceExpression new];
-	
-	// First, we check that the operands and operator are not nil. If they are,
-	// then the expression is invalid...
-	if (leftHandExpression == nil ||
-		rightHandExpression == nil ||
-		operator == SA_DiceExpressionOperator_NONE) {
-		expression.type = SA_DiceExpressionTerm_NONE;
-		expression.errorBitMask |= SA_DiceExpressionError_INVALID_EXPRESSION;
-		return expression;
-	}
-	
-	// If the operands and operator are present, then the expression is an
-	// operation expression...
-	expression.type = SA_DiceExpressionTerm_OPERATION;
-	
-	// ... but does it have a valid operator?
-	if (operator == SA_DiceExpressionOperator_MINUS ||
-		operator == SA_DiceExpressionOperator_PLUS ||
-		operator == SA_DiceExpressionOperator_TIMES) {
-		expression.operator = operator;
-	} else {
-		expression.errorBitMask |= SA_DiceExpressionError_UNKNOWN_OPERATOR;
-		return expression;
-	}
-	
-	// The operator is valid. Set the operands...
-	expression.leftOperand = leftHandExpression;
-	expression.rightOperand = rightHandExpression;
-
-	// And inherit any errors that they may have.
-	expression.errorBitMask |= expression.leftOperand.errorBitMask;
-	expression.errorBitMask |= expression.rightOperand.errorBitMask;
-
-	// Since this top-level expression was NOT generated by parsing an input
-	// string, for completeness and consistency, we have to generate a fake
-	// input string ourselves! We do this by wrapping each operand in
-	// parentheses and putting the canonical representation of the operator
-	// between them.
-	expression.inputString = [NSString stringWithFormat:@"(%@)%@(%@)",
-							  expression.leftOperand.inputString,
-							  [SA_DiceFormatter canonicalRepresentationForOperator:expression.operator],
-							  expression.rightOperand.inputString];
-
-	// The joining is complete. (Power overwhelming.)
-	return expression;
-}
-
 /**********************************************/
 #pragma mark - “Legacy” behavior implementation
 /**********************************************/
@@ -234,10 +186,9 @@ static NSDictionary *_validCharactersDict;
 			// expression type fall through to the remaining cases (roll command
 			// or simple value).
 			// In the latter case, we register an error and return.
-			if (dieRollString.length == lastOperatorRange.length ||
-				![[SA_DiceParser validCharactersForOperator:SA_DiceExpressionOperator_MINUS] containsCharactersInString:operator]) {
+			if (   dieRollString.length == lastOperatorRange.length
+				|| ([[SA_DiceParser validCharactersForOperator:SA_DiceExpressionOperator_MINUS] containsCharactersInString:operator] == NO)) {
 				SA_DiceExpression *expression = [SA_DiceExpression new];
-
 				expression.type = SA_DiceExpressionTerm_OPERATION;
 				expression.inputString = dieRollString;
 				expression.errorBitMask |= SA_DiceExpressionError_INVALID_EXPRESSION;
@@ -304,10 +255,10 @@ static NSDictionary *_validCharactersDict;
 	if ([[SA_DiceParser validCharactersForOperator:SA_DiceExpressionOperator_PLUS] containsCharactersInString:operatorString]) {
 		// Check to see if the term is an addition operation.
 		expression.operator = SA_DiceExpressionOperator_PLUS;
-	} else if([[SA_DiceParser validCharactersForOperator:SA_DiceExpressionOperator_MINUS] containsCharactersInString:operatorString]) {
+	} else if ([[SA_DiceParser validCharactersForOperator:SA_DiceExpressionOperator_MINUS] containsCharactersInString:operatorString]) {
 		// Check to see if the term is a subtraction operation.
 		expression.operator = SA_DiceExpressionOperator_MINUS;
-	} else if([[SA_DiceParser validCharactersForOperator:SA_DiceExpressionOperator_TIMES] containsCharactersInString:operatorString]) {
+	} else if ([[SA_DiceParser validCharactersForOperator:SA_DiceExpressionOperator_TIMES] containsCharactersInString:operatorString]) {
 		// Check to see if the term is a multiplication operation.
 		// Look for other, lower-precedence operators to the left of the
 		// multiplication operator. If found, split the string there 
@@ -351,8 +302,6 @@ static NSDictionary *_validCharactersDict;
 	// These roll one or more dice of a given sort, and determine the sum of
 	// their rolled values. (In the “exploding dice” version, each die can
 	// explode, of course.)
-	// In the future, support for other, more complex roll commands might be 
-	// added, such as “roll several and return the highest”.
 	if ([[SA_DiceParser validCharactersForRollCommandDelimiter:SA_DiceExpressionRollCommand_SUM]
 		 containsString:[dieRollString substringWithRange:delimiterRange]])
 		expression.rollCommand = SA_DiceExpressionRollCommand_SUM;
@@ -453,6 +402,7 @@ static NSDictionary *_validCharactersDict;
 	}
 }
 
+// TODO: Should this be on a per-mode, and therefore per-instance, basis?
 +(NSString *) allValidCharacters {
 	return [ @[ [SA_DiceParser validNumeralCharacters], 
 				[SA_DiceParser allValidRollCommandDelimiterCharacters],