Factory Method implementation

Can you please verify my approach?

using System;

    /*
    * In Factory pattern, we create object without exposing the creation logic. 
    * In this pattern, an interface is used for creating an object,
    * but let subclass decide which class to instantiate.
    * */

namespace FactoryMethod
{

Product (abstract)

/*
    * Faza Class ITree
    *
    * */
    public interface ITree
    {
        string GetTreeName();
    }

Products (concrete)

/*
    * The Concrete class which implements ITree
    * 
    * */

    public class BananaTree : ITree
    {
        public string GetTreeName()
        {
            return "My Name Is Banana Tree";
        }
    }

    /*
    * The Concrete class which implements ITree
    * 
    * */
    public class CoconutTree : ITree
    {
        public string GetTreeName()
        {
            return "My Name Is Coconut Tree";
        }
    }

Factory (abstract)

/*
    * Faza Class TreeType 
    * If you want you can add abstract class instad of faza class
    *
    * */
    public interface TreeType
    {
        ITree GetTree(string tree);
    }

Factory (concrete)

/*
    * Concrete class which implements faza or concrete class
    *
    * */
    public class ConcreteTreeType : TreeType
    {

        public ITree GetTree(string tree)
        {

            if (tree == "COCONUT")
                return new CoconutTree();
            else
                return new BananaTree();

        }
    }

Client code

/*
    * main app.
    * 
    *  */
    class Program
    {
        static void Main(string[] args)
        {
            TreeType oTreeType = new ConcreteTreeType();

            ITree banana = oTreeType.GetTree("COCONUT");
            Console.WriteLine(banana.GetTreeName());

            Console.ReadKey();
        }
    }
}

I personally would not rely on using magic strings.


Magic strings are where you have taken something like a class/method/variable name and written it within a string, which is then used to identify the appropriate class/method/variable

This makes it hard to refactor later on if you change a class name, it is too easy to miss instances etc. Furthermore, the code that you currently have is case-sensitive. This might be by design, however, consider the following:

var actuallyABananaTree = oTreeType.GetTree("Coconut");

Even though the developer is specifying a coconut tree, he actually gets a banana tree. If the construction logic was changed to:

if (tree.Equals("coconut", StringComparison.OrdinalIgnoreCase))

You would then get the right tree type back. See StringComparison Enum for the comparison options.

In that same construction logic, you are also not checking to see whether or not the parameter tree is a null reference. This might be something that you wish to consider adding, especially if you use the approach suggested above.

Use of Enums

A better approach would be to use enums instead of magic strings. For example:

enum MyTreeTypes {
      Coconut,
      Banana
}

Then in your construction logic, you can have:

public ITree GetTree(MyTreeTypes tree)
{
    switch (tree)
    {
        case MyTreeTypes.Coconut:
            return new CoconutTree();
        default:
            return new BananaTree();
    }
}

Using this approach ensures type safety and prevents problems of magic strings when refactoring etc. Also, you will compile time errors if you spell the tree type incorrectly

StackExchange Code Review Q#48308, answer score: 11