What's the way to pass data to a method
Let's say we have 2 classes Expense1
and Expense2
. Which implementation of the class is preferred over the other, or which is considered closer to object-oriented?
I always thought that execution was Exp2.Calculate(1.5M,2)
more readable than
exp1.Calculate()
and using the properties of the exp1 class as the required values for the calculation method.
Expense1
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
public class Expense1
{
public decimal ExpenseValue { get; set; }
public int NumberOfItems { get; set; }
private decimal result;
public decimal Result
{
get
{
return this.NumberOfItems * this.ExpenseValue;
}
}
public void Calculate()
{
this.result = this.ExpenseValue * this.NumberOfItems;
}
public void expense1()
{
this.ExpenseValue = 0;
this.NumberOfItems = 0;
}
}
Expense2
class Expense2
{
public decimal Calculate(decimal expenseValue, int numberOfItems)
{
return expenseValue * numberOfItems;
}
}
Implementation of both classes
class Program
{
static void Main(string[] args)
{
Expense1 exp1 = new Expense1();
exp1.NumberOfItems = 2;
exp1.ExpenseValue = 1.5M ;
exp1.Calculate();
Console.WriteLine("Expense1:" + exp1.Result.ToString());
Expense2 exp2 = new Expense2();
string result = string.Empty;
result = exp2.Calculate(1.5M,2).ToString();
Console.WriteLine("Expense2:" + result);
Console.ReadKey();
}
}
source to share
Expense2
it's easier to read and understand what's going on as you can see from the call whose parameters are being used.
Expense1
can have various side effects because the caller may forget to set the variables used for the computation before he calls Calculate()
.
If you need access to the values that were used to calculate the result later, you can use something like this
public class Expense {
public decimal ExpenseValue { get; private set; }
public int NumberOfItems { get; private set; }
public decimal Result { get; private set; }
public decimal Calculate(decimal expenseValue, int numberOfItems) {
ExpenseValue = expenseValue;
NumberOfItems = numberOfItems;
Result = ExpenseValue * NumberOfItems;
return Result;
}
public Expense() {
Calculate(0, 0);
}
}
This will allow the internal state Expense
to remain consistent for the lifetime of the object with the definition of how itCalculated
source to share
Expense2.Calculate
is deterministic (it has the same result every time it is called with the same parameters) and because it has no side effects (parameters are passed to it, not through properties), it is also thread safe. Finally, it is easier and easier to read.
Expense1
- a classic OOP cruise with a safe state without a ceiling and does not guarantee that it Result
will return the result of calling the thread Calculate
. Plus, it is durable and difficult to read and maintain.
I would approve Expense2
more Expense1
every time. The only thing I would change is to make it static as there is no need to instantiate it Expense2
to call Calculate
.
source to share
I don't like Expense1 one bit. I think that properties with a public get / set should generally be avoided as it doesn't provide encapsulation. It's hardly even better than the public fields (although it does give some room for reservations for guards, etc. On the way).
Even worse is the Calculate () imo method. It takes no arguments, but it still changes the internal state of the object. It is not clear how. At the same time, the field it writes is result
not returned by the property result
, this property repeats the same computation (if you need to do computations, usually prefer method over property).
Finally, there is no need to initialize primitive types to their default values in the constructor, number types are always initialized to 0.
===
Expense2 is better, it's clearer what's going on. The method takes two well-known arguments and gives a reasonable result right away.
If this method is the only use case for the class, I would consider renaming it, since it does not represent an expense, it is something akin to a cost calculator.
source to share
It all depends on what you want to do, but a rule of thumb is usage properties when they are actually associated with the object and pass what to use as parameters when they are external to the object.
Simple example, assuming Person is a class, one of its properties is birthday, so in this case you should do this
public class Person{
public DateTime Birthday{get;set;}
public int GetAge(){
//some implementation
}
}
another example, think about having a Cube object and in that Cube object you have an Intersect method with another cube, in this case you have to pass the second cube as a parameter and not make it a Cube property because it is something external.
public class Cube{
public Cube Intersect(Cube other){
//Do your logic here
}
}
source to share