1
\$\begingroup\$

(Follow-up on RPG Character Builder)

This implements a fluent interface for RPG game characters. Is this a valid implementation?

There are 3 characters: Paladin, Wizard and Elfo. There are 3 items: Potion, Sword, Arch.

[TestFixture]
public class RPGEngineTest
{
    [Test]
    public void GivenAPaladin_WithSword_Return()
    {
        //Arrange
        var paladin = CharacterInterface
                        .Create(new Paladin())
                        .Named("Conan")
                        .WithSword()
                        .Build();

        //Act
        var result = paladin.Atack();

        //Assert                    
        Assert.AreEqual("Conan has split the enemy in two", result);
    }

    [Test]
    public void GivenAPaladin_WithArch_Return()
    {
        //Arrange
        var paladin = CharacterInterface
                   .Create(new Paladin())
                   .Named("Conan")
                   .WithArch()
                   .Build();

        //Act
        var result = paladin.Atack();

        //Assert                    
        Assert.AreEqual("Conan has hit the enemy's heart", result);
    }

    [Test]
    public void GivenAnElfo_WithArch_Return()
    {
        //Arrange
        var elfo = CharacterInterface
                  .Create(new Elfo())
                  .Named("Légolas")
                  .WithArch()
                  .Build();

        //Act
        var result = elfo.Atack();

        //Assert                    
        Assert.AreEqual("Légolas has hit the enemy's heart", result);
    }

    [Test]
    public void GivenAnElfo_WithPotion_Return()
    {
        //Arrange
        var elfo = CharacterInterface
                  .Create(new Elfo())
                  .Named("Légolas")
                  .WithPotion()
                  .Build();

        //Act
        var result = elfo.Atack();

        //Assert                    
        Assert.AreEqual("Légolas has converted the enemy into a monkey", result);
    }
    [Test]
    public void GivenAMago_WithPotion_Return()
    {
        //Arrange
        var mago = CharacterInterface
                  .Create(new Mago())
                  .Named("Merlín")
                  .WithPotion()
                  .Build();

        //Act
        var result = mago.Atack();

        //Assert                    
        Assert.AreEqual("Merlín has converted the enemy into a monkey", result);
    }
    [Test]
    public void GivenAMago_WithSword_Return()
    {
        //Arrange
        var mago = CharacterInterface
                  .Create(new Mago())
                  .Named("Merlín")
                  .WithSword()
                  .Build();

        //Act
        var result = mago.Atack();

        //Assert                    
        Assert.AreEqual("Merlín has split the enemy in two", result);
    }
}   

public interface IItem
{
    string Use();
}
public class Sword : IItem
{
    public string Use()
    {
        return "{0} has split the enemy in two";
    }
}
public class Arch : IItem
{
    public string Use()
    {
        return "{0} has hit the enemy's heart";
    }
}
public class Potion : IItem
{
    public string Use()
    {
        return "{0} has converted the enemy into a monkey";
    }
}   
public interface IAction
{
    string Atack();
}
public interface ICharacter: IAction
{
    IItem Item { get; set; }
    string Name { get; set; }        
}
public class Paladin : ICharacter
{
    public IItem Item { get; set; }
    public string Name { get; set; }      
    public string Atack()
    {
        return string.Format(Item.Use(), Name);
    }
}
public class Elfo : ICharacter
{
    public IItem Item { get; set; }
    public string Name { get; set; }
    public string Atack()
    {
        return string.Format(Item.Use(), Name);
    }
}
public class Mago : ICharacter
{
    public IItem Item { get; set; }
    public string Name { get; set; }
    public string Atack()
    {
        return string.Format(Item.Use(), Name);
    }
}

public interface IName
{
    IAddItem Named(string name);
}
public interface IAddItem
{
    IBuild WithArch();
    IBuild WithSword();
    IBuild WithPotion();
}
public interface IBuild
{
    ICharacter Build();
}  

public class CharacterInterface : IName, IAddItem, IBuild
{
    private ICharacter _character;
    private CharacterInterface(ICharacter character)
    {
        _character= character;
    }

    public static IName Create(ICharacter character)
    {
        return new CharacterInterface(character);
    }

    public IAddItem Named(string name)
    {
        _character.Name = name;
        return this;
    }
    public IBuild WithArch()
    {
        _character.Item = new Arch();
        return this;
    }

    public IBuild WithSword()
    {
        _character.Item = new Sword();
        return this;
    }

    public IBuild WithPotion()
    {
        _character.Item = new Potion();
        return this;
    }

    public ICharacter Build()
    {
        return _character;
    }
}
\$\endgroup\$
  • \$\begingroup\$ The correct spelling is "Attack", not "Atack". I also don't understand why you use personaje when the rest of your code is in English. Why not use character? \$\endgroup\$ – BCdotWEB Aug 14 '15 at 15:24
2
\$\begingroup\$

While it may be a "valid" implementation, it is not the best, and could stand for several improvements. I've outlined a couple.


This bit:

public class Paladin : ICharacter
{
    public IItem Item { get; set; }
    public string Name { get; set; }      
    public string Atack()
    {
        return string.Format(Item.Use(), Name);
    }
}
public class Elfo : ICharacter
{
    public IItem Item { get; set; }
    public string Name { get; set; }
    public string Atack()
    {
        return string.Format(Item.Use(), Name);
    }
}
public class Mago : ICharacter
{
    public IItem Item { get; set; }
    public string Name { get; set; }
    public string Atack()
    {
        return string.Format(Item.Use(), Name);
    }
}

screams for an abstract or base class.

public abstract class Character : ICharacter
{
    public IItem Item { get; set; }
    public string Name { get; set; }

    public string Attack()
    {
        return string.Format(Item.Use(), Name);
    }
}

public class Paladin : Character
{
}

public class Elfo : Character
{
}

public class Mago : Character
{
}

This will centralize the common methods/properties/events/etc. with all your characters, meaning, you only have to write the implementation once.

The only downfall is that you can only inherit one other class, any other "inheritance" must be implementing interfaces. That said, your abstract class can also inherit one other class and other interfaces as well.


Another suggestion: pass your Character (or ICharacter) object to the Item.Use() method. This helps significantly as, at some point, you are likely to want to affect other characters. Create an abstract class for it and abstract away the Use() method.

public interface IItem
{
    string Use(Character source, Character destination);
}

public abstract class Item : IItem
{
    protected string _usageText;

    public virtual string Use(Character target)
    {
        // For items that only affect the current character, and no others.
        return string.Format(_usageText, target.Name);
    }

    public virtual string Use(Character source, Character destination)
    {
        // For items that affect another character, i.e. attacking items
        return string.Format(_usageText, source.Name);
    }
}

public class Sword : Item
{
    public Sword()
    {
        _usageText = "{0} has split the enemy in two";
    }

    public override string Use(Character source, Character destination)
    {
        base.Use(source, destination);

        // subtract hit-points from destination Character here if desired
    }
}

public class Arch : Item
{
    public Arch()
    {
        _usageText = "{0} has hit the enemy's heart";
    }
}

public class Potion : Item
{
    public Potion()
    {
        _usageText = "{0} has converted the enemy into a monkey";
    }
}

Then, rewrite Character as:

public abstract class Character : ICharacter
{
    public IItem Item { get; set; }
    public string Name { get; set; }

    public string Attack(Character target)
    {
        return Item.Use(this, target);
    }
}

The nice thing about this, is that you can override the Use(Character target) and Use(Character source, Character destination) methods within the sub-classes of Item, to do other things specific to that item.

The problem with this implementation method is that it could be a bit difficult to separate out the strings. Having multiple Use methods like this that all rely on the same string is less-than-ideal. There are surely other ways to do this which are more effective, but this is what I could come up with fairly quickly.

\$\endgroup\$

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.