Trying to implement cross-class functionality that needs a field to operate in C#

Question

Crosspost from here, it was suggested that this is better place to pose my question.

---

I'm implementing a series of classes in a game that all derive from one base class that references the scripting of the object that it pertains. Most of these are objects that are generated by certain spells/abilities, and very frequently there is a lot of overlap between functionality each script needs.

Unfortunately, the functionality requires the use of some kind of field to use, whether it be a Dictionary or a couple of primitives. Normally I'd just apply an interface to each one, but because each "piece of functionality" requires a field to operate correctly that doesn't seem optimal. And adding every kind of multi-use functionality into the base class seems like it would make each object unnecessarily bloated.

Here's an example:

BlazingAura_Server

...
public BlazingAuraC_Server conditionInstance;
public Queue<KeyValuePair<int, float>> reapplyDelay = new Queue<KeyValuePair<int, float>>();
...
void Update()
{
 KeyValuePair<int, float> id;
 while (reapplyDelay.Count > 0)
 {
 if (reapplyDelay.Peek().Value - BlazingAuraC_Server.BURNING_REAPPLY_DELAY > conditionInstance.duration)
 {
 id = reapplyDelay.Dequeue();
 }
 else
 {
 break;
 }
 }
}
void OnTriggerEnter(Collider collider)
{
 if (collider.tag != "Spell")
 {
 EntityInfoServer collidedInfo = collider.transform.GetComponentInChildren<EntityInfoServer>();
 collidedInfo.QueueDamageRaw(conditionInstance, BlazingAuraC_Server.TICK_FREQ * BlazingAuraC_Server.BASE_DPS, ValueModDamageSubtype.Elemental);
 if (!reapplyDelay.Any(id => id.Key == collidedInfo.entityId))
 {
 reapplyDelay.Enqueue(new KeyValuePair<int, float>(collidedInfo.entityId, conditionInstance.duration));
 collider.transform.GetComponentInChildren<EntityInfoServer>().AddConditionServer(Condition.Burning, conditionInstance);
 }
 } 
}

And here's another example:

Storm_Server

...
public float duration;
public Queue<KeyValuePair<int, float>> reapplyDelay = new Queue<KeyValuePair<int, float>>();
...
void Update()
{
 duration += Time.deltaTime;
 if (Mathf.CeilToInt((duration + Time.deltaTime) / TICK_FREQ) > Mathf.CeilToInt(duration / TICK_FREQ))
 {
 Activate(stormCollider);
 }
 KeyValuePair<int, float> id;
 while (reapplyDelay.Count > 0)
 {
 if (reapplyDelay.Peek().Value - STORM_DRENCH_DELAY > duration)
 {
 id = reapplyDelay.Dequeue();
 }
 else
 {
 break;
 }
 }
}
public void OnTriggerEnter(Collider collider)
{
 if (collider.tag == "Player")
 {
 //Debug.Log("IN STORM");
 EntityInfoServer collidedInfo = collider.transform.GetComponentInChildren<EntityInfoServer>();
 if (!reapplyDelay.Any(id => id.Key == collidedInfo.entityId))
 {
 reapplyDelay.Enqueue(new KeyValuePair<int, float>(collidedInfo.entityId, duration));
 collider.transform.GetComponentInChildren<EntityInfoServer>().AddConditionServer(Condition.Drenched, stormInfo);
 }
 }
}

Basically, each class keeps track of entities that have collided with it and store it in a queue, and make sure that a condition doesn't get reapplied unless the entity has been dequeued after a certain duration. However, it requires the Queue to work correctly, but declaring a Queue and utilizing an interface in each class doesn't seem like very good abstraction and feels sloppy in general.

What are my best options for including functionality like this (i.e. having a reapply delay) across multiple classes?

• mdfst13: Our rules are different from Stack Overflow's. See A guide to Code Review for Stack Overflow users. As written, SO would probably tell you that this is too much code. We'd say that it is too little.
• I'll add comments tomorrow: I have a hard time understanding your code. Could you give a walkthrough as to what would typically happen one after another? It looks like you want to apply some conditions that last only for a certain duration. But why do they get "reapplied"?

Answer 1

I think the rule you should follow here is "favor composition over inheritance".

What that means in your case is that you create a separate class, something like ReapplyDelay and make both classes own an instance of that class.

The code could look something like this:

class ReapplyDelay
{
 private Queue<KeyValuePair<int, float>> reapplyDelay = new Queue<KeyValuePair<int, float>>();
 public void RemoveOld(float delay, float duration)
 {
 while (reapplyDelay.Count > 0)
 {
 if (reapplyDelay.Peek().Value - delay > duration)
 {
 reapplyDelay.Dequeue();
 }
 else
 {
 break;
 }
 }
 }
 public bool AddNew(EntityInfoServer collidedInfo, float duration)
 {
 if (!reapplyDelay.Any(id => id.Key == collidedInfo.entityId))
 {
 reapplyDelay.Enqueue(new KeyValuePair<int, float>(collidedInfo.entityId, duration));
 return true;
 }
 return false;
 }
}

And your two original classes would then look like this:

BlazingAura_Server:

private BlazingAuraC_Server conditionInstance;
private ReapplyDelay reapplyDelay = new ReapplyDelay();
void Update()
{
 reapplyDelay.RemoveOld(BlazingAuraC_Server.BURNING_REAPPLY_DELAY, conditionInstance.duration);
}
void OnTriggerEnter(Collider collider)
{
 if (collider.tag != "Spell")
 {
 EntityInfoServer collidedInfo = collider.transform.GetComponentInChildren<EntityInfoServer>();
 collidedInfo.QueueDamageRaw(conditionInstance, BlazingAuraC_Server.TICK_FREQ * BlazingAuraC_Server.BASE_DPS, ValueModDamageSubtype.Elemental);
 if (reapplyDelay.AddNew(collidedInfo, conditionInstance.duration))
 {
 collider.transform.GetComponentInChildren<EntityInfoServer>().AddConditionServer(Condition.Burning, conditionInstance);
 }
 } 
}

Storm_Server:

private float duration;
private ReapplyDelay reapplyDelay = new ReapplyDelay();
void Update()
{
 duration += Time.deltaTime;
 if (Mathf.CeilToInt((duration + Time.deltaTime) / TICK_FREQ) > Mathf.CeilToInt(duration / TICK_FREQ))
 {
 Activate(stormCollider);
 }
 reapplyDelay.RemoveOld(STORM_DRENCH_DELAY, duration);
}
public void OnTriggerEnter(Collider collider)
{
 if (collider.tag == "Player")
 {
 //Debug.Log("IN STORM");
 EntityInfoServer collidedInfo = collider.transform.GetComponentInChildren<EntityInfoServer>();
 if (reapplyDelay.AddNew(collidedInfo, duration))
 {
 collider.transform.GetComponentInChildren<EntityInfoServer>().AddConditionServer(Condition.Drenched, stormInfo);
 }
 }
}

You know your code better than I do, so the details might be different, but the general idea should be clear: extract common independent pieces of functionality into a separate class and then use that.

I have also changed your fields from public to private: if you really need to expose them, you should use public properties, not fields.

• Kyton: This is the solution I ended up sticking with as well; thanks so much for outlining it so clearly!