Jump to content

  • Log In with Google      Sign In   
  • Create Account


Inheritance, Interfaces, and code duplication


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
13 replies to this topic

#1 SolarChronus   Members   -  Reputation: 199

Like
5Likes
Like

Posted 27 March 2013 - 10:46 AM

Hey everyone,

 

I am trying to make sure I am writing my classes in the best manner possible (program to an interface not an implementation, using good oop practices, open-closed principle, etc). I believe I am on the right track with that I am doing, but I have question as to a specific part in my class, as I feel like I will be writing duplicate code between classes.

 

I have a class called MissionEncounter which implemented the IEncounter interface. The code is below.

 

public class MissionEncounter : IEncounter<Mission>
{
    private RandomizedList<Mission> missions = new RandomizedList<Mission>();
    private string path = "Assets/Resources/XML Files/Activities/Missions";

    public MissionEncounter()
    {
        string[] files = Reader.GetXmlFiles(path);
        Create(files, missions);
        missions.Shuffle();
    }

    public Mission GetNext()
    {
        return missions.Next();
    }

    private void Create(string[] files, RandomizedList<Mission> list)
    {
        for (int i = 0; i < files.Length; i++)
        {
            //...use XML to retrieve Encounter(Mission in this case) specific information
            //...create a new Mission(...)
            //...add new Mission to the missions list
        }
    }
}
public interface IEncounter<T>
{
    T GetNext();
    void Create(string[] files, RandomizedList<T> list);
}

 

 

I will have multiple types of encounters in my game, some are missions, some are shops, random loot, neutral/hostile ships, and more. I was thinking I had to create a new ________Encounter class for each type of encounter. (LootEncounter, ShopEncounter, etc). The reason for this is that each encounter types details are defined by an XML file structure and there are multiples of each (100 missions, 3 shops, etc). Using the XML format allows me to easily add new content without having to write new code - unless I want to create a totally new encounter type.

 

What is bugging me is that the _______Encounter class would all be duplicates of each other except for the code inside the method that creates the encounter type. Create(string[] files, RandomizedList<T> list).

 

I was trying to stay away from inheritance as much as possible, but would it be acceptable in this situation to have MissionEncounter inherit from some type of generic base class that implements IEncounter so that it takes care of class construction, the GetNext() method, and implements an overridable Create(..) method?



Sponsor:

#2 SiCrane   Moderators   -  Reputation: 9497

Like
3Likes
Like

Posted 27 March 2013 - 11:14 AM

Another option is one class that has multiple CreateBlahBlahBlah() functions and you pass the constructor an enum or something similar to select which one is called when the Create() function is called. Yet another would be have your Create() function identify what kind of encounter type is being created from the XML and call the appropriate create implementation automatically without the caller needing to know exactly what kind of encounter is encoded inside the XML.

#3 Zipster   Crossbones+   -  Reputation: 580

Like
0Likes
Like

Posted 27 March 2013 - 12:18 PM

It seems to me like you'd want to move your mission creation to an entirely separate factory class that knows how to parse the XML data for each individual mission type and construct them by passing the relevant data directly to their constructors. That way missions know nothing about XML or how they came to be, just that they now exist and have appropriate data.

 

This will also allow you to remove the Create() function from your IEncounter interface, which doesn't really seem to belong there. I inferred that "encounters" handle mission sequencing from the GetNext() method, but also having them create the missions gives them multiple responsibilities and violates SRP.



#4 SolarChronus   Members   -  Reputation: 199

Like
0Likes
Like

Posted 27 March 2013 - 12:29 PM

@Zipster I think some confusion is a product of bad class naming on my part. The MissionEncounter class would be considered the factory for loading all the mission xml data into a list of missions and then handing one out when requested. There is an actual Mission class which only houses the data for that mission, title text, description, messages, choices, etc. Same with the shop, loot, and ship classes.



#5 SeraphLance   Members   -  Reputation: 1305

Like
2Likes
Like

Posted 27 March 2013 - 12:33 PM

I would probably do something along the lines of this:

 

public class Encounter : IEncounter
{
    private RandomizedList<Encounterable> list = new RandomizedList<Encounterable>();
    private EncounterStrategy strategy;

    public Encounter(EncounterStrategy strategy)
    {
        string[] files = Reader.GetXmlFiles(strategy.getPath());
        this.strategy = strategy;
        Create(files, list);
        list.Shuffle();
    }

    public Encounterable GetNext()
    {
        return list.Next();
    }

    private void Create(string[] files, RandomizedList<Encounterable> list)
    {
        for (int i = 0; i < files.Length; i++)
        {
            //...use XML to retrieve Encounter(Mission in this case) specific information
            //the strategy creates the proper Encounterable.
            //add Encounterable to list.
        }
    }
}


public interface EncounterStrategy
{
    string getPath();
    //other methods required to create encounterable.
}

 

 

This way all of the unique functionality is in one place (MissionEncounterStrategy, LootEncounterStrategy, etc.) instead of forcing you to inherit and keep overriding create methods.

 

EDIT:  lots of little code errors.


Edited by SeraphLance, 27 March 2013 - 12:35 PM.


#6 SolarChronus   Members   -  Reputation: 199

Like
0Likes
Like

Posted 27 March 2013 - 12:53 PM

@SeraphLance

 

Interesting concept there, but I am left wondering something. Each encounterable will have differing data. Missions (title, mission text, choices, and outcomes) differs from a Shop encounterable (dealer name, type of items they sell, stock). So the lines of code in the create method for the XML portion would be drastically different as they are targeting different elements and attributes. 

 

I'm leaning towards SiCranes response, about having the Create method choose the XML method to use to grab the information out of the file. I'm just worried I'll end up creating smelly switch statements. I understand if used properly (once and only once for a certain procedure) they are fine, but I typically over use them in some areas and they've been a pain in the butt to modify when I've had major additions, and the fact I haven't been able to grasp the polymorphic alternative to switch statements. Most examples are so simple I have a hard time translating how to apply them to my code.



#7 rozz666   Members   -  Reputation: 613

Like
0Likes
Like

Posted 27 March 2013 - 12:57 PM

It's hard to decide how to split the responsibilities of your classes. Try making a domain model (http://en.wikipedia.org/wiki/Domain_model) to define your dictionary and find relation between your abstractions, and post it.



#8 SeraphLance   Members   -  Reputation: 1305

Like
1Likes
Like

Posted 27 March 2013 - 01:07 PM

@SeraphLance

 

Interesting concept there, but I am left wondering something. Each encounterable will have differing data. Missions (title, mission text, choices, and outcomes) differs from a Shop encounterable (dealer name, type of items they sell, stock). So the lines of code in the create method for the XML portion would be drastically different as they are targeting different elements and attributes. 

 

I'm leaning towards SiCranes response, about having the Create method choose the XML method to use to grab the information out of the file. I'm just worried I'll end up creating smelly switch statements. I understand if used properly (once and only once for a certain procedure) they are fine, but I typically over use them in some areas and they've been a pain in the butt to modify when I've had major additions, and the fact I haven't been able to grasp the polymorphic alternative to switch statements. Most examples are so simple I have a hard time translating how to apply them to my code.

 

That's what the strategy is for.  The strategy implementation stores a path and knows how to read the files from that path to create encounterables.  One "could" argue that it's a violation of the SRP to do both, but I'd say they're sufficiently connected tasks.  The strategy pattern basically is the polymorphic alternative to switch statements.

 

It's worth noting that the whole strategy.getPath() thing is kind of stupid, but I put it in there as a sort of minimum-modification example.  Really, the create function in this case would just have the strategy handle all that.



#9 SolarChronus   Members   -  Reputation: 199

Like
0Likes
Like

Posted 27 March 2013 - 01:20 PM

@Serpah.

Aha! I see what you mean now. Ill try that out when Im back at my computer.

As for breaking SRP, I have a static xml class that retrieves data for me in all the major types and allows casting to custom types via some simple method invokes. Im not actually writing xml reader code in the create method, so SRP should be mostly satified.

#10 Madhed   Crossbones+   -  Reputation: 2710

Like
0Likes
Like

Posted 27 March 2013 - 02:34 PM

If I understand the code correctly, I am seeing 2 problems here. The Encounter class does too many things and Mission is only vaguely defined.

 

Encounter currently handles Mission creation and Mission enumeration.

 

You are saying, there will be different kind of Missions with different data. Would it be possible to make Mission a base class and extend from there different Mission types? Maybe it would even be possible to model all the different scenarious by making Mission a container for "MissionObjectives".

 

Then you can move the creation logic out of the Encounter class and create a list of Mission objects which are filled by factories based on the xml supplied.



#11 SolarChronus   Members   -  Reputation: 199

Like
0Likes
Like

Posted 27 March 2013 - 02:46 PM

If I understand the code correctly, I am seeing 2 problems here. The Encounter class does too many things and Mission is only vaguely defined.

Encounter currently handles Mission creation and Mission enumeration.

You are saying, there will be different kind of Missions with different data. Would it be possible to make Mission a base class and extend from there different Mission types? Maybe it would even be possible to model all the different scenarious by making Mission a container for "MissionObjectives".

Then you can move the creation logic out of the Encounter class and create a list of Mission objects which are filled by factories based on the xml supplied.



#12 SolarChronus   Members   -  Reputation: 199

Like
0Likes
Like

Posted 27 March 2013 - 02:52 PM

Its actually a product of some bad naming choices. Mission is only vaguly defined because its a concrete data class. The MissionEncounter class is what loads all the mission data files into a list. Im going to be changing the class names a bit to reflect their usage better.

Im going to adopt seraphs approach and see how that works out. With a few alterations, it looks like the polymorphic way I was searching for.

#13 SolarChronus   Members   -  Reputation: 199

Like
2Likes
Like

Posted 27 March 2013 - 04:29 PM

Ok! I think I have a handle on this now, and it seems to do exactly as I want and is easily extendable! Here is my code now.

 

 

I created a generic EncounterFactory to be able to handle any type of encounter I want. Even though all the encounters inherit from a base Encounterable, I still need to spit out concrete version(Mission, Loot, etc).

public class EncounterFactory<T>
{
    private RandomizedList<T> randomizedList = new RandomizedList<T>();
    private IEncounterStrategy<T> strategy;

    public EncounterFactory(IEncounterStrategy<T> strategy)
    {
        this.strategy = strategy;
        string[] files = Reader.GetXmlFiles(strategy.GetPath());
        strategy.Create(files, randomizedList);
        randomizedList.Shuffle();
    }

    public T GetNext()
    {
        return randomizedList.Next();
    }
}

 

This is the generic IEncounterStrategy interface. Pretty simple.

public interface IEncounterStrategy<T>
{
    void Create(string[] files, RandomizedList<T> list);
    T GetNext();
    string GetPath();
}

 

Here is an example of the MissionStrategy class for loading up all the mission encounters from the xml files. All I need to do is create a new strategy class for each encounter type I want to add. This should make it easy for me to add new encounter types later on.

public class MissionStrategy : IEncounterStrategy<Mission>
{
    private string path = "Assets/Resources/XML Files/Activities/Missions";

    public MissionStrategy()
    {
    }

    public void Create(string[] files, RandomizedList<Mission> randomizedList)
    {
     // for (int i = 0; i < files.Length; i++)
     // {
     //      ...some code to extract information from the xml file
     //      Mission mission = new Mission(...all the data extracted from the xml file...)
     //      randomizedList.Add(mission);
     // }
    }

    public string GetPath()
    {
        return path;
    }
}

 

And here is how I create the factory & get a new mission out of the factory.

        EncounterFactory<Mission> missionFactory = new EncounterFactory<Mission>(new MissionStrategy());
        missionFactory.GetNext();

 

 

I'm pretty happy with the result. Hopefully I did this right, as it seems to work fine!



#14 NightCreature83   Crossbones+   -  Reputation: 2703

Like
2Likes
Like

Posted 28 March 2013 - 10:49 AM

You are completely on the right track there just a few hints though you don't need to create a Factory to do the factory patterns construction functions are enough, as long as they are globally available.

This is my way of approaching this problem if you are interested in seeing it. It uses the reflection stuff that C# gives me to create the objects and after that the objects just take over the deserialisation of the node to initialise themselves from.
public class UIEntityReader : IIncludeFileReader
    {
        public UIEntityReader(String fileName, Game game) : base(fileName)
        {
            m_game = game;
        }

        protected override void processCurrentFile()
        {
            XmlDocument xmlDocument = new XmlDocument();
            xmlDocument.Load(m_stream);

            XmlNode node = xmlDocument.FirstChild.NextSibling.FirstChild;

            while (node != null)
            {
                if (node is XmlNode && !(node is XmlComment))
                {
                    if (node.Name == "_include")
                    {
                        //We have just found another file we need to process for entity reading
                        XmlAttribute fileAttribute = node.Attributes["file"];
                        if (fileAttribute != null)
                        {
                            addFile(Directory.GetCurrentDirectory() + "\\Content\\ui\\screens\\" + fileAttribute.Value);
                        }
                    }
                    else
                    {
                        //Node should be a Entity
                        Entity entity = ObjectFactory.createEntity(m_game, node);
                        entity.Active = false;
                        entity.deserialise(node);
                        if (node.HasChildNodes)
                        {
                            XmlNode childNode = node.FirstChild;
                            while (childNode != null)
                            {
                                //Child nodes are either Attributes or Components
                                if (childNode.Name.Contains("Attribute"))
                                {
                                    IAttributeHandle handle;
                                    DebugWrapper.Assert(childNode.Attributes["name"] != null, "Attribute xml node has no \"name\" attribute" );
                                    String attributeName = childNode.Attributes["name"].Value;
                                    if (!entity.HasAttribute(attributeName))
                                    {
                                        handle = ObjectFactory.CreateAttributeHandle(m_game, childNode);
                                        entity.AddAttribute(handle);
                                    }
                                    else
                                    {
                                        handle = entity.getAttribute(attributeName);
                                    }

                                    if (handle != null)
                                    {
                                        handle.deserialise(childNode);
                                    }
                                }
                                else if (childNode.Name.Contains("Component"))
                                {
                                    ComponentManager compManRef = ComponentManager.getInstance(m_game);
                                    ComponentHandle temp = compManRef.GetComponent(childNode.Name);
                                    if (temp == null)
                                    {
                                        //Component doesn't exist yet so create one and add it to the manager
                                        Component component = ObjectFactory.CreateComponent(m_game, childNode);
                                        DebugWrapper.Assert(component != null, "Failed to create component: {0}", childNode.Name);
                                        component.deserialise(childNode);
                                        compManRef.AddComponent(new ComponentHandle(childNode.Name, component));
                                    }
                                    entity.AddComponent(childNode.Name);
                                }
                                else
                                {
                                    DebugWrapper.Warn("Found a child node with an unrecognized type {0}", childNode.Name);
                                }

                                childNode = childNode.NextSibling;
                            }
                        }

                        GameObjectManager.getInstance(m_game).AddGameObject(entity);
                    }

                    node = node.NextSibling;
                }
                else
                {
                    node = node.NextSibling;
                }
            }
        }

        private Game m_game;
    }

public class ObjectFactory
    {
        public static object constructObjectFromString(Game game, string typeName)
        {
            var type = getReflectionType(typeName);
            if (type != null)
            {
                return createGameObjectFromType(game, type);
            }

                return null;
        }

        public static Object constructObjectFromXmlNode(Game game, XmlNode node)
        {
            var type = getReflectionType(node);

            if (type != null)
            {
                return createGameObjectFromType(game, type);
            }

            return null;
        }

        public static Entity createEntity(Game game, XmlNode node)
        {
            var entityType = getReflectionType(node);
            if (entityType == null) return null;

            return (Entity)createOneParamObjectFromType<Game>(game, entityType);
        }

        public static Component CreateComponent(Game game, XmlNode node)
        {
            var componentType = getReflectionType(node);
            if (componentType == null) return null;

            return (Component)createOneParamObjectFromType<Game>(game, componentType);
        }

        public static IAttributeHandle CreateAttributeHandle(Game game, XmlNode node)
        {
            var attributeType = getReflectionType(node);
            if (attributeType == null) return null;

            Object obj = createZeroParamObjectFromType(attributeType);
            if (UtilFunctions.isDerivedType(typeof(IAttributeHandle), obj.GetType()))
            {
                return (IAttributeHandle)obj;
            }

            return null;
        }

        public static IAttributeHandle CreateAttributeHandle(Game game, Type genericType)
        {
            String typeName = "AttributeHandle";
            switch (Type.GetTypeCode(genericType))
            {
                case TypeCode.Int32:
                    typeName += "Int";
                    break;
                case TypeCode.Single:
                    typeName += "Float";
                    break;
                case TypeCode.Boolean:
                    typeName += "Bool";
                    break;
                case TypeCode.Byte:
                    typeName += "Byte";
                    break;
                case TypeCode.Char:
                    typeName += "Char";
                    break;
                case TypeCode.String:
                    typeName += "String";
                    break;
            }

            Type typeToConstruct = getReflectionType(typeName);
            if (typeToConstruct != null)
            {
                return (IAttributeHandle)createZeroParamObjectFromType(typeToConstruct);
            }

            return null;
        }

        public static ComponentHandle createComponentHandle(String type)
        {
            Type typeToConstruct = getReflectionType(type);
            return (ComponentHandle)createZeroParamObjectFromType(typeToConstruct);
        }

        public static Model constructModelFromFileName(Game game, Type modelType, Type modelReader, String fileName)
        {
            var modelObject = modelType != null ? createGameObjectFromType(game, modelType) : null;
            if (modelObject != null)
            {
                var modelReaderObject = modelReader != null ? createOneParamObjectFromType<IModelReader, String>(fileName, modelReader) : null;
                if (modelReaderObject != null)
                {
                    return modelReaderObject.loadFile(game);
                }
                else
                {
                    //Assume this is a XNA content pipeline object and load it in that way
                    ContentManager content = new ContentManager(game.Services, "Content");
                    return new FBXModel(content.Load<Microsoft.Xna.Framework.Graphics.Model>(fileName), game);
                }
            }
            return null;
        }

        public static object createGameObjectFromType(Game game, Type type)
        {
            return createOneParamObjectFromType<Game>(game, type);
        }

        public static Object constructGenericParamType(Type objectType, Type genericType)
        {
            Type[] typeArgs = { genericType };
            Type constructed = objectType.MakeGenericType(typeArgs);
            return  Activator.CreateInstance(constructed);
        }

        public static Object createZeroParamObjectFromType(Type type)
        {
            Type[] constructorParams = { };
            var constructor = type.GetConstructor(constructorParams);
            Object[] constructionParams = { };
            return constructor.Invoke(constructionParams);
        }

        public static Object createOneParamObjectFromType<T>(T obj, Type type)
        {
            Type[] constructorParams = { typeof(T) };
            var constructor = type.GetConstructor(constructorParams);
            Object[] constructionParams = { obj };
            return constructor.Invoke(constructionParams);
        }

        public static ReturnT createOneParamObjectFromType<ReturnT, T>(T obj, Type type)
        {
            Type[] constructorParams = { typeof(T) };
            var constructor = type.GetConstructor(constructorParams);
            Object[] constructionParams = { obj };
            return (ReturnT)constructor.Invoke(constructionParams);
        }

        public static Object createTwoParamObjectFromType<T, S>(T obj, S obj2, Type type)
        {
            Type[] constructorParams = { typeof(T), typeof(S) };
            var constructor = type.GetConstructor(constructorParams);
            Object[] constructionParams = { obj, obj2 };
            return constructor.Invoke(constructionParams);
        }

        public static ReturnT createTwoParamObjectFromType<ReturnT, T, S>(T obj, S obj2, Type type)
        {
            Type[] constructorParams = { typeof(T), typeof(S) };
            var constructor = type.GetConstructor(constructorParams);
            Object[] constructionParams = { obj, obj2 };
            return (ReturnT)constructor.Invoke(constructionParams);
        }

        public static Object createThreeParamObjectFromType<T, S, R>(T obj, S obj2, R obj3, Type type)
        {
            Type[] constructorParams = { typeof(T), typeof(S), typeof(R) };
            var constructor = type.GetConstructor(constructorParams);
            Object[] constructionParams = { obj, obj2, obj3 };
            return constructor.Invoke(constructionParams);
        }

        private static Type getReflectionType(string typeName)
        {
            var type = Type.GetType(typeName);
            if (type == null)
            {
                type = Type.GetType("SpaceSimulator." + typeName);
            }
            return type;
        }

        private static Type getReflectionType(XmlNode node)
        {
            return getReflectionType(node.Name);
        }
    }

Edited by NightCreature83, 28 March 2013 - 10:50 AM.

Worked on titles: CMR:DiRT2, DiRT 3, DiRT: Showdown, GRID 2, Mad Max




Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS