1

In my code, I've got two interfaces, let's say IOpenable and IExaminable. IOpenable allows the user to Open or Close the object, and IExaminable allows the user to Examine the object.

Now, we have some sort of Tile structure which contains objects which adhere to the following: Not all objects are openable, not all objects are examinable, some are both.

When the user right clicks a Tile in-map, I want to populate some sort of list with the possible targets and their varied functionalities.

As a concrete example, let's say that a Book is examinable and openable, whereas a Card is just examinable. When I right click, I wanted to make it so that I can navigate through Open -> Book or Examine -> [Card | Book].

At the moment, I'm essentially copy/pasting code, where I assess each time if any object is IOpenable and add it to a list of Open targets. Then I do the same for Close, then IExaminable with Examine. I'm tying the functionality of the button to a delegate e.g. [Open -> Book] would have the functionality

delegate 
{
    lockedInteractable.Open(); 
}

The problem with this is, I'm reusing the same code every single time, just changing which interface we're looking for (e.g. changing IOpenable to IExaminable) and changing the function in the delegate (e.g. .Open() to .Examine() or whatever).

Is there a more efficient way to do this? Is this simply a product of my bad code practice in making things implement IOpenable and IExaminable like that?

Clarification

In my setup, Opening results in a shift of sprites, much like closing (I do not want to make this a toggle functionality) and examining actually takes a string in and outputs some flavour text about the object, e.g. "[Bob] examined the [Door], and found it to be very disinteresting."

The duplication of code is essentially as follows:

I start by creating an instance of Button for each examinable. Button then has the on click event linked to .Examine for that examinable object. We then make an instance of Button that opens the menu with all the previously made buttons, and has the text examine. By doing this, we setup a menu with Examine -> Target as described before.

I then create an instance of Button for each openable. Button then has the on-click event linked to .Open for that...

etc for each possible operation and function.

I was hoping to find a way to simply pass in, say, IExaminable or IOpenable and what part I want made into a menu, e.g. .Open and automatically create a menu, rather than repeatedly duplicating the code and just changing the method and interface.

Code

if(tileInteractable is ILock)
{
        ILock lockedInteractable = tileInteractable as ILock;
        if(lockedInteractable.Locked)
        {
            int lockedItemCount = 0;
            List<string> lockedStrings = new List<string>();
            List<Action> lockedActions = new List<Action>();
            lockedItemCount++;
            lockedStrings.Add (tileInteractable.ObjectName);
            lockedActions.Add(new Action(
                                delegate 
                                {
                                lockedInteractable.Unlock (GameManager.instance.player.entity); 
                                Reset();
                                }
                            ));
            radComponentCount++;
            buttonStrings.Add ("Unlock");
            actions.Add(new Action(
                                delegate 
                                {
                                Reset();
                                GenerateRadial(lockedItemCount,lockedActions,lockedStrings);
                                isActive = true;
                                }
                            ));
        }
}

To run you through the code, the following occurs:

A radial menu button is made. It's told to create one button with the entity's name as its hover text.

When pressed, it calls Unlock with the sender as the player, and then closes the radial menu via Reset().

We then create another radial button labelled Unlock that runs the other radial menu when it's pressed. This radial button is then built into a base menu with any other similar ones, so the first menu has Unlock, Examine, etc. if anything exists that implements that functionality, and the second refers to the potential targets.

This code is then nearly identically duplicated, with ILock changed to IExaminable and .Unlock changed to .Examine. Doing this creates the Examine functionality for the main radial menu.

  • 1
    What code exactly are you duplicating? Also, I don't think there is anything wrong with your design. I could maybe suggest better design if I knew more about what exactly it means to open or examine something. – Euphoric Jul 26 '15 at 8:56
  • I concur with @Euphoric. The use of the two interfaces sounds sensible and, even with your clarification, it's hard to understand why you are having to duplicate code. Please share some of this duplicated code so we can better understand what you are trying to do. – David Arno Jul 26 '15 at 9:31
  • Re-IExamine your design. I'm seeing the Command Pattern, events, and aspect oriented coding, and Popeil Dependency Injector, all pureed in an attempt to get a Unified Field Theory smoothie. @IdanArye hits the nail on the head I think - "dynamic mechanism ... to infer actions..." It's like you're trying to build an arbitrarily abstract framework. – radarbob Jul 27 '15 at 19:54
2

Interfaces are a static tool that allows programmers to guarantee what methods an object supports.

You are trying to use them as a dynamic mechanism for allowing users - or, more precisely, the user interface - to infer the actions users are allowed to do on the the object.

What you need is a uniform way to programmatically access the list of actions at runtime, so that the code creating the UI will be able to iterate over the allowed actions and created buttons for them without caring what each specific action does.

There are two ways to achieve that:

  1. Don't use IOpenable and IExaminable, and instead have each object(or object type) hold a table of allowed actions, that you can register manually.

  2. Use reflection. Basically you want to somehow mark the methods Open, Close and Examine, and have the UI use reflection to get these methods and convert them to buttons for the user. The marking can be done in many ways(depending on what the language supports) - you can depend on the signature, use annotations, use special names, use companion members, place the marking on the interfaces the methods originate from... as long as you can distinguish the user-action methods without specifying each one in the UI generation code.

  • I'm not entirely sure I understand option 1. Suppose we have each object type hold a table of allowed actions - would we still not at some point need to find the relevant method from it? For example, if our "book" object type was marked as openable, we still at some point would need to cast from our generic object stored in a tile to a book and then run that.Open() the moment we actually want to interact with it, no? Wouldn't that essentially boil down to the previous code, just instead of doing it only for "IOpenable" we'd have to do it for every openable object? – user188644 Jul 26 '15 at 17:19
  • You don't need casting in option 1. The book will have in it's table values under the keys "Open" and "Examine". These values can be delegates, or objects, or whatever - but they should all be invokable in the same way, so both Open and Examine are invoked via an Invoke method(doesn't have to be named "Invoke". It can be Perform or the call operator or whatever). The UI doesn't care what Invoke does behind the scenes in each case - all it cares is that it has a label to put on the button and an Invoke method it can call when the user clicks the button. – Idan Arye Jul 26 '15 at 22:37
  • I think I understand better now - thanks for your patience and your clear explanation. I think I will opt for an alternate build that uses option 1), and see if I can get it working. It seems promising as a general method of approach - with this idea, am I correct in saying we would want either a base class or some interface Interactable that says "This object has an action table", to avoid having to get each specific class' unique action table? – user188644 Jul 27 '15 at 2:18
  • Yup - without a base class/interface for objects that have an action table(and for objects that can be placed in Tile), you won't be able to treat them uniformly in the UI builder code. – Idan Arye Jul 27 '15 at 14:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.