World resource system for Adventure Game/RPG

Question

I'm beginning the long process of developing an Adventure Game/RPG and I was hoping to get some review on a resource system I'm coming up with. How it works is, a resource is generic to every resource in the world. Each resource will have a name, a random yield, and a random quality. So I've created an interface for the generic properties and function each resource will have.

Interface

public interface IResource{
    string Name {get;set;}
    string Quality {get;set;}
    int Yield{get;set;}
    string ConvertQualityToString();
    int GetResourceYield();
}

Then I implement the interface in an actual resource base class.

Implementation

public class Resource : MonoBehaviour, IResource{
    public string Name { get; set; }
    public string Quality { get; set; }
    public int Yield { get; set; }

    public string ConvertQualityToString(){
        var val = Random.Range(0, 3);
        string quality;
        switch (val){
            case 1:
                quality = "Good";
                break;
            case 2:
                quality = "Great";
                break;
            default:
                quality = "Poor";
                break;
        }
        return quality;
    }

    public int GetResourceYield(){
        var val = Random.Range(1, 25);
        return val;
    }
}

And finally, I create the specific resource itself and attach it to my game object in the inspector.

Resource Class

public class Granite : Resource{
    void Start(){
        Name = "Granite";
        Quality = ConvertQualityToString();
        Yield = GetResourceYield();
    }
}

Is this the proper way to work this system? Obviously, the system actually does work as when I log the values on startup, Yield and Quality are randomized and the Name for each resource object I create is correct. Is there a better way to do this? Are there things I should change? Thanks in advance.

Answer 1

A few notes:

• All IResource properties have public setters. Are you absolutely certain that other code should be able to modify those? I'd recommend making these read-only unless you have specific reasons not to.
• ConvertQualityToString and GetResourceYield look like internal helper methods. Making them part of the IResource interface creates a very confusing API: what's the difference between resource.Yield and resource.GetResourceYield()? With the way the code is structured, I'd expect these methods to be protected.
• Are you sure that string is a good type for Quality? If you've got a fixed number of qualities an enum is a more strongly-typed and descriptive choice. And if different qualities have different characteristics then perhaps a Quality class makes more sense. Try to avoid writing 'stringly-typed' code.
• ConvertQualityToString gives a 33% chance for each quality. Likewise, GetResourceYield always produces yields between 1 and 25. Are you sure that's applicable to all resources and in all circumstances? Should quality and yield even be determined by a Resource instance itself? What if, for example, some enemies should have a higher chance of dropping high-quality resources? Or what if you want to provide player upgrades that increase the average resource yield?
• What's the point of that Granite class? There's no custom logic in there, and the resource name itself is sufficient to distinguish between different resources.
• In ConvertQualityToString, there's no need for that local quality variable: you can let each case return a value directly (which also allows you to remove those break statements).
• ConvertQualityToString is a strange name: it doesn't really convert anything. Inaccurate and confusing names tend to make code more difficult to work with.