tl;dr– If you need to share a mutable state between different methods in the same class, then having that mutable state stored in a field/property seems entirely reasonable. However, you may want to centralize how that state is read/modified rather than having each method duplicating the same access logic. To do this, you may want to write up a simple dispatcher.
It sounds like you've got a property that's being mutated by different methods in the same class:
public class SomeObject
{
private DateTime LastMethodCallTime { get; set ; }
// Methods that read/write to the above property:
public void SomeMethod_001() { /* ... */ }
public void SomeMethod_002() { /* ... */ }
public void SomeMethod_003() { /* ... */ }
}
Having a shared state isn't really a bad thing when it's unavoidable. This is, if you need to have coordination between method calls, then there necessarily must be something providing that coordination. That part's fine.
But why a DateTime
? Does each method need to properly implement the same behavior for checking/updating it? Because if so, then that's probably something to fix.
Instead, you might want to code it sorta like:
public class SomeObject
{
private DateTime LastMethodCallTime { get; set ; }
private object ___LOCK___lastMetodCallTimeLockObject { get; } = new object();
protected void PerformTimeSensitiveAction(Action<DateTime> actionToPerform)
{
lock (this.___LOCK___lastMetodCallTimeLockObject)
{
var lastMethodCallTime = this.LastMethodCallTime;
actionToPerform(lastMethodCallTime);
this.LastMethodCallTime = DateTime.Now;
}
}
// Methods that call this.PerformTimeSensitiveAction():
public void SomeMethod_001() { /* ... */ }
public void SomeMethod_002() { /* ... */ }
public void SomeMethod_003() { /* ... */ }
}
Or, you could segregate the dispatcher logic to avoid having too much in the same class:
public class SomeObject
{
private SimpleDispatcher Dispatcher { get; } = SimpleDispatcher.New();
// Methods that call this.Dispatcher:
public void SomeMethod_001() { /* ... */ }
public void SomeMethod_002() { /* ... */ }
public void SomeMethod_003() { /* ... */ }
}
public class SimpleDispatcher
{
private DateTime LastMethodCallTime { get; set ; }
private object ___LOCK___lastMetodCallTimeLockObject { get; } = new object();
protected SimpleDispatcher() { }
public static SimpleDispatcher New()
{
var toReturn = new SimpleDispatcher();
return toReturn;
}
public void Perform(
Action<DateTime> actionToPerform
)
{
lock (this.___LOCK___lastMetodCallTimeLockObject)
{
var lastMethodCallTime = this.LastMethodCallTime;
actionToPerform(lastMethodCallTime);
this.LastMethodCallTime = DateTime.Now;
}
}
public T_Result Perform<T_Result>(
Func<DateTime, T_Result> funcToPerform
)
{
lock (this.___LOCK___lastMetodCallTimeLockObject)
{
var lastMethodCallTime = this.LastMethodCallTime;
var toReturn = funcToPerform(lastMethodCallTime);
this.LastMethodCallTime = DateTime.Now;
return toReturn;
}
}
}
Usually an object that controls method accesses like this is called a dispatcher. For example, if you write GUI code (e.g., in WPF), you'll often need to call a dispatcher to perform actions within a GUI thread.
In a case like this, you might not care much about how the dispatcher works, in which case a C#-lock
might be sufficient. Or if you want finer behavior, you might implement, e.g., a dispatcher queue using a ConcurrentQueue<Action>
.
Whatever the case, the fact that you have a mutable field/property shared across your object doesn't appear to be a problem since, if you need to share state between different methods, that's a pretty logical place to put it. Instead, the concern'd be if you're duplicating the logic that works with that object; so, instead of using a plain DateTime
and duplicating how to access it, writing a simple dispatching method or class would seem cleaner.