patterncsharpMinor
Implementation of a coin jar
Viewed 0 times
coinimplementationjar
Problem
Implement a coin jar in C#. The coin jar will only accept US coinage
and has a volume of 32 fluid ounces. Additionally, the jar has a
counter to keep track of the total amount of money collected and has
the ability to reset the count back to $0.00.
I received the question above from one of the recruiter for initial screening. He asked me to submit code for it, which I did and pasted below.
Please review and show me any improvements that I can make to my code.
```
//Please ignore coins’ volume, as they are not from valid sources.
public interface ICoinJar
{
void Accept(ICoin coin);
IVolume TotalVolume { get; }
ICurrency ActualAmount { get; }
IVolume ActualVolume { get; }
void Reset();
}
public interface ICoin
{
IVolume Volume { get; }
ICurrency Value { get; }
}
public interface IVolume
{
long Unit { get; set; }
double InRelativeMeasure();
}
public interface ICurrency
{
long UnitPrice { get; set; }
double InCurrency();
}
///
/// For this class, One unit price is equal to One cent.
/// For other
///
public class USCurrency : ICurrency
{
public long UnitPrice { get; set; }
public double InCurrency()
{
return UnitPrice / 100;
}
}
public class FluidOunces : IVolume
{
public long Unit
{
get;
set;
}
///
/// Considering Unit is 10000 times smaller than one Fluid Ounce.
///
///
public double InRelativeMeasure()
{
return (double)this.Unit / 10000;
}
}
public abstract class UsCoin : ICoin
{
public string Owner
{
get
{
return "Federal Reserve";
}
}
public abstract IVolume Volume { get; }
public abstract ICurrency Value { get; }
}
public class Penny : UsCoin
{
private IVolume volume;
private ICurrency currency;
public Penny()
{
volume = new FluidOunces();
volume.Unit = 122;
currency = new USCurrency();
currency.Unit
and has a volume of 32 fluid ounces. Additionally, the jar has a
counter to keep track of the total amount of money collected and has
the ability to reset the count back to $0.00.
I received the question above from one of the recruiter for initial screening. He asked me to submit code for it, which I did and pasted below.
Please review and show me any improvements that I can make to my code.
```
//Please ignore coins’ volume, as they are not from valid sources.
public interface ICoinJar
{
void Accept(ICoin coin);
IVolume TotalVolume { get; }
ICurrency ActualAmount { get; }
IVolume ActualVolume { get; }
void Reset();
}
public interface ICoin
{
IVolume Volume { get; }
ICurrency Value { get; }
}
public interface IVolume
{
long Unit { get; set; }
double InRelativeMeasure();
}
public interface ICurrency
{
long UnitPrice { get; set; }
double InCurrency();
}
///
/// For this class, One unit price is equal to One cent.
/// For other
///
public class USCurrency : ICurrency
{
public long UnitPrice { get; set; }
public double InCurrency()
{
return UnitPrice / 100;
}
}
public class FluidOunces : IVolume
{
public long Unit
{
get;
set;
}
///
/// Considering Unit is 10000 times smaller than one Fluid Ounce.
///
///
public double InRelativeMeasure()
{
return (double)this.Unit / 10000;
}
}
public abstract class UsCoin : ICoin
{
public string Owner
{
get
{
return "Federal Reserve";
}
}
public abstract IVolume Volume { get; }
public abstract ICurrency Value { get; }
}
public class Penny : UsCoin
{
private IVolume volume;
private ICurrency currency;
public Penny()
{
volume = new FluidOunces();
volume.Unit = 122;
currency = new USCurrency();
currency.Unit
Solution
Personally, I would just do away with a lot of the code. That would include most interfaces and most classes, as they don't really do anything towards solving the task at hand.
Anyway, you have two classes that use integer storage internally, but only expose the value as a double, so there is no advantage in storing it other than as a double.
You can use the constructor to set the value, that way it's not possible to create one without initialising it properly, and you can keep it from being changeable once it's set.
In all your coin classes you repeat the same code for storage and getters, so you can just put that in the base class. Again you can use the constructor to easily initialise the class, and make sure that it's not possible to inherit the base class without properly initialising it.
Anyway, you have two classes that use integer storage internally, but only expose the value as a double, so there is no advantage in storing it other than as a double.
You can use the constructor to set the value, that way it's not possible to create one without initialising it properly, and you can keep it from being changeable once it's set.
public class USCurrency : ICurrency {
public double UnitPrice { get; private set; }
public UsCurrency(double unitPrice) {
UnitPrice = unitPrice;
}
}
public class FluidOunces : IVolume {
public double Unit { get; private set; }
public FluidOunces(double unit) {
Unit = unit;
}
}In all your coin classes you repeat the same code for storage and getters, so you can just put that in the base class. Again you can use the constructor to easily initialise the class, and make sure that it's not possible to inherit the base class without properly initialising it.
public abstract class UsCoin : ICoin {
public IVolume Volume { get; private set; }
public ICurrency Value { get; private set; }
public UsCoin(double volume, double value) {
Volume = new FluidOunces(volume);
Value = new USCurrency(value);
}
}
public class Penny : UsCoin {
public Penny() : base(0.0122, 0.01) { }
}
public class Nickel : UsCoin {
public Nickel() : base(0.0243, 0.05) { }
}
public class Dime : UsCoin {
public Dime() : base(0.0115, 0.1) { }
}
public class Quarter : UsCoin {
public Quarter() : base(0.0270, 0.25) { }
}
public class HalfDollar : UsCoin {
public HalfDollar() : base(0.0534, 0.5) { }
}
public class Dollar : UsCoin {
public Dollar() : base(0.0800, 1.0) { }
}Code Snippets
public class USCurrency : ICurrency {
public double UnitPrice { get; private set; }
public UsCurrency(double unitPrice) {
UnitPrice = unitPrice;
}
}
public class FluidOunces : IVolume {
public double Unit { get; private set; }
public FluidOunces(double unit) {
Unit = unit;
}
}public abstract class UsCoin : ICoin {
public IVolume Volume { get; private set; }
public ICurrency Value { get; private set; }
public UsCoin(double volume, double value) {
Volume = new FluidOunces(volume);
Value = new USCurrency(value);
}
}
public class Penny : UsCoin {
public Penny() : base(0.0122, 0.01) { }
}
public class Nickel : UsCoin {
public Nickel() : base(0.0243, 0.05) { }
}
public class Dime : UsCoin {
public Dime() : base(0.0115, 0.1) { }
}
public class Quarter : UsCoin {
public Quarter() : base(0.0270, 0.25) { }
}
public class HalfDollar : UsCoin {
public HalfDollar() : base(0.0534, 0.5) { }
}
public class Dollar : UsCoin {
public Dollar() : base(0.0800, 1.0) { }
}Context
StackExchange Code Review Q#32757, answer score: 8
Revisions (0)
No revisions yet.