Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Order.Management/Order.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

namespace Order.Management
{

/* I think Report HAS A Order rather than Report IS A Order would be better */
abstract class Order
{
public string CustomerName { get; set; }
Expand Down
3 changes: 3 additions & 0 deletions Order.Management/PaintingReport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ public override void GenerateReport()
generateTable();
}

//too public, exposing implementation details
//this class knows too much about shapes, if just knew about abstract idea of shape and told it to print common behaviour would be easy to add another shape implementation in future
//at the moment all clients of shapes would need to be updated (Painting Report Invoice Report Cutting List Report) This is breaking Open Closed Principle
public void generateTable()
{
PrintLine();
Expand Down
10 changes: 9 additions & 1 deletion Order.Management/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,32 @@

namespace Order.Management
{
// this class is too big and uncohesive should be decomposed
class Program
{
// Main entry
static void Main(string[] args)
{
/* static methods are procedural, would be better to be decomposed into objects instead so can mock easier */
var (customerName, address, dueDate) = CustomerInfoInput();

var orderedShapes = CustomerOrderInput();


/* reports could be abstracted to share common interface */
InvoiceReport(customerName, address, dueDate, orderedShapes);

CuttingListReport(customerName, address, dueDate, orderedShapes);

PaintingReport(customerName, address, dueDate, orderedShapes);
}


/* these comments are just noise and arent actually helpful, should be deleted */
// Order Circle Input
public static Circle OrderCirclesInput()
public static Circle OrderCirclesInput()//too public
{
/* probably shouled be DRYed up into an abstraction and use polymorphism */
Console.Write("\nPlease input the number of Red Circle: ");
int redCircle = Convert.ToInt32(userInput());
Console.Write("Please input the number of Blue Circle: ");
Expand Down Expand Up @@ -110,6 +117,7 @@ private static (string customerName, string address, string dueDate) CustomerInf
// Get order input
private static List<Shape> CustomerOrderInput()
{
//would loosely couple if was talking to an interface like IShape instead of these concretions
Square square = OrderSquaresInput();
Triangle triangle = OrderTrianglesInput();
Circle circle = OrderCirclesInput();
Expand Down
10 changes: 6 additions & 4 deletions Order.Management/Shape.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,22 @@

namespace Order.Management
{
//interface would be better (composition over inheritance)
abstract class Shape
{
//instance vars are too public and have no immutability
public string Name { get; set; }
public int Price { get; set; }
public int AdditionalCharge { get; set; }
public int NumberOfRedShape { get; set; }
public int AdditionalCharge { get; set; } //should probably hide implementation detail
public int NumberOfRedShape { get; set; }// I would suggest using decorater pattern something like IShape shape = new RedShape(new SquareShape(new Shape(3)));
public int NumberOfBlueShape { get; set; }
public int NumberOfYellowShape { get; set; }
public int TotalQuantityOfShape()
public int TotalQuantityOfShape() // would consider making shape smart enough to print itself in report rather than client of shape asking for quantity and then have client responsibile for printing it
{
return NumberOfRedShape + NumberOfBlueShape + NumberOfYellowShape;
}

public int AdditionalChargeTotal()
public int AdditionalChargeTotal()// additional charge should probably be encapsulated away and only visible behaviour is price value
{
return NumberOfRedShape * AdditionalCharge;
}
Expand Down