HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Model class representing a calculator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
classmodelcalculatorrepresenting

Problem

I have a simple Model class that represents a calculator.

Header file:

@class MMCalculator;

@protocol MMCalculatorDelegate 

@required - (void)mmCalculator:(MMCalculator *)mmCalulatorFinishedCalculatingValues;

@end

@interface MMCalculator : NSObject

@property (nonatomic, weak) id  delegate;
@property float calculatedPay;
@property float calculatedSavingsForStuff;
@property float calculatedSavingsForProfitFormula;
@property float calculatedSavingsForTaxes;

- (void)calculateValuesWithMonthlyRevenue:(float)monthlyRevenue;
- (float)calculateYourPay:(float)monthlyRevenue;
- (float)calculateSavingsForStuff:(float)monthlyRevenue;
- (float)calculateSavingsForProfitFormula:(float)monthlyRevenue;
- (float)calculateSavingsForTaxes:(float)monthlyRevenue;

@end


Implementation file:

```
#import "MMCalculator.h"

#pragma mark - Static Variables
static float const percentageToPayYourself = 0.50f;
static float const percentageToSaveForStuff = 0.20f;
static float const percentageToSaveForProfitFormula = 0.20f;
static float const percentageToSaveForTaxes = 0.10f;

@implementation MMCalculator

#pragma mark - Calculator Methods
  • (void)calculateValuesWithMonthlyRevenue:(float)monthlyRevenue


{
//Call all 4 calculation methods and set calculated properties
self.calculatedPay = [self calculateYourPay:monthlyRevenue];
self.calculatedSavingsForStuff = [self calculateSavingsForStuff:monthlyRevenue];
self.calculatedSavingsForProfitFormula = [self calculateSavingsForProfitFormula:monthlyRevenue];
self.calculatedSavingsForTaxes = [self calculateSavingsForTaxes:monthlyRevenue];

//Call delegate method
[self.delegate mmCalculator:self];
}
  • (float)calculateYourPay:(float)monthlyRevenue


{
return percentageToPayYourself * monthlyRevenue;
}

  • (float)calculateSavingsForStuff:(float)monthlyRevenue


{
return percentageToSaveForStuff * monthlyRevenue;;
}

  • (float)calculateSavingsForProfitFormula:(float)monthlyRevenue


{
return percentageToSaveForP

Solution

Let's address your questions:

-
I don't think that storing the calculated values in public properties is a good idea, mainly because If I call calculateValuesWithMonthlyRevenue with say, 1000, and then afterwards call calculateYourPay with 400, the other properties don't change to reflect my different monthly revenue. Therefore, I'm not sure why these should be stored as properties at all. I would eliminate calculateValuesWithMonthlyRevenue, and make this class a singleton with only static methods.

-
In most cases, static constants are written in ALLCAPS or ALL_CAP, depending on your prefered style. I would define these in your header file (if you want these values to be public) or in your implementation file (if you want the values to be private) with #define instead, but that's a matter of personally preference.

-
You don't show us what you do with this delegate method, so I can't exactly be sure what the point of it is. I would omit it for now.

-
Using floats is a bit dangerous as it lends itself to annoying floating-point errors such as .3 turning into .299999999. This is really a question of desired precision. If you're fine with being off by a bit, then floats are pretty decent. If not, you will have to define your own type to represent money.

Context

StackExchange Code Review Q#60522, answer score: 4

Revisions (0)

No revisions yet.