is this Good Design? OOP Loose Couple Principle?
hello! i want to show you if my Design is "correct" according to OOP principles.
info :
there is a ball which collects coins
the ball is the player.
there is a game manager
so before you see the Scripts i've made a graph :
i've made a Skill Class :
using UnityEngine;
using System.Collections;
public class Skill {
public int Duration { get; set; }
SkillType Type;
public enum SkillType
{
Unknown,
Shield,
Life,
}
//Constructor
public Skill(int _duration,SkillType _type)
{
Duration = _duration;
Type = _type;
}
public SkillType getType()
{
return Type;
}
}
then there is a coin prefab that has attached CoinSkill script on it
using UnityEngine;
using System.Collections;
public class CoinSkill : MonoBehaviour {
Skill ShieldSkill = new Skill(10, Skill.SkillType.Shield);
GameCoreManager gameManager;
// Use this for initialization
void Start () {
Destroy(gameObject, ShieldSkill.Duration);
gameManager = GameObject.FindWithTag("GameController").GetComponent<GameCoreManager>();
}
void OnCollision2D(Collision2D collider)
{
if(collider.gameObject.tag == "Player")
{
gameManager.CollectCoin(ShieldSkill.getType(),collider.gameObject);
Destroy(gameObject);
}
}
}
& GameCoreManager script attached to GameManager Object
using UnityEngine;
using System.Collections;
public class GameCoreManager : MonoBehaviour {
public void CollectCoin(Skill.SkillType _type,GameObject player)
{
if(_type == Skill.SkillType.Shield)
{
player.GetComponent<SkillActivator>().ActivateSkill(_type);
}
}
}
and finally the skillactivator script attached to player (ball) object
using UnityEngine;
using System.Collections;
public class SkillActivator : MonoBehaviour {
// Use this for initialization
void Start () {
}
// Update is called once per frame
void Update () {
}
public void ActivateSkill(Skill.SkillType _type)
{
if(_type == Skill.SkillType.Shield)
{
activateShieldSkill();
}
}
public void activateShieldSkill()
{
//todo
}
}
Answer by RasmusDyhr · Jul 02, 2016 at 12:37 AM
I would probably encapsulate the skills more. Have the skill object store the method to be activated, to decouple it completely from the SkillActivator.
I however feel that the GameManager is an issue in its current state as it doesn't do anything. You could just as easily call the SkillActivator class from the CoinSkill class. And if you refactor out the GameManager and follow my first advice of encapsulating the Skill logic in the Skill class, then the SkillActivator becomes redundant, and should be refactored away as well.
The bottom line is that you're risking fragmenting rather than decoupling your code, if you just do it for the sake of "good design".
thank you man!i am just trying to improve to good design!