I feel like these constants should be in a different class?

Question

I have a static class called RenderingUtilities that houses several useful methods and constants. Some of these constants are related to the Earth as an object such as the Earth's radius. I believe the constants related to the Earth should be contained in the Earth class, but have conflicting thoughts on it since the RenderingUtilities class shouldn't really depend on other objects.

The reason for this is that the Earth class is an object that can be rendered to the screen. Thus it has a base class of RenderObject and has properties and methods on its own. It will also rely on the RenderingUtilities class to retrieve a geometric primitive in order to render. Thus this seems like circumnavigation to me.

However, my counter argument to my own counter argument is that since these are const values and are accessed without creating an instance of the Earth class, that it should be okay.

---

Rendering Utilities Class

The RenderingUtilities class is a collection of helpful methods with the following categories:

• Conversions
  • Simple conversions such as degrees to radians.
  • Computer coordinate conversions such as world to screen.
  • Coordinate conversions such as ECR to ECI.
  • Earth projections such as Polar, Mercator, etc.
• Assistive Rendering
  • Simplifying the process of rendering text.
  • Simplifying the process of rendering 2D polygons.

A very simple example method is the conversion from degrees to radians:

public static double ToRadians(this double degrees) => degrees * Math.PI / 180.0;

This allows quick access through the double type, along with accessing it through the static class:

double radians = RenderingUtilities.ToRadians(45);
double degrees = 90;
radians = degrees.ToRadians();

There are more, but this should help add some clarity as to what the RenderingUtilities class is helping with. There is no need to create individual classes for these helpful methods, and they are used quite often, such as rendering text that displays the position of the camera for debugging, or frame rate, or error messages, etc. There are practical applications such as rendering detailed information about a specific point on the Earth. However, the methods are generic enough that you supply a screen position, some text, and a color, and it displays it.

---

Render Object Class

The RenderObject class is an abstract base class that is home to the fundamentals of each object rendered to the screen; such as:

public abstract class RenderObject {

    #region Fields

    private List<RenderObject> children = new List<RenderObject>();

    #endregion

    #region Properties

    public bool Active { get; set; } = false;
    public string Name { get; set; } = string.Empty;
    public RenderObject Parent { get; protected set; } = null;
    public RenderObject[] Children => children.ToArray();

    #endregion

    #region Public Methods

    public abstract void Initialize();
    public abstract void Update();
    public abstract void Render();

    #endregion

}

---

Earth Class

This class is derived from the class RenderObject (see above). It is responsible for, well, rendering the Earth. This includes local instances of classes (also deriving from RenderObject) such as:

• Gridlines
• Terrain
• Water
• etc.

The flow of the code there is very object oriented and that is what caused me to come here and pose this question.

---

The Current Placement of the Constants

The reason these constants are kept within the RenderingUtilities class is due to the extensive use of those constants in conversion methods mentioned above. However, since they are related to the Earth per se, I believe it may make more sense to move them there. For example:

// Doesn't make much sense.
RenderingUtilities.EARTH_RADIUS_IN_METERS

// Makes more sense.
Earth.RADIUS_IN_METERS

---

The Question

Is there an issue with regards to OOP practices that would prevent me from putting these constants in the Earth class and using them in the static RenderingUtilities class?

Answer 1

I see two immediate solutions for your problem:

• If you cannot refactor too much, bite the bullet and move physical constants into another class - not the Earth class, but, for example, a class named PhysicalConstants, which carries only what the name says. That will avoid the cyclic dependency, and the constants will then be in a "neutral" place, where classed like Earth and RenderingUtilities can use them both. Yes, RenderingUtilities will loose it's selfcontainment that way - but you can't have the cake and eat it.

• My preferred approach: check which methods in RenderingUtilities precisely use those "earth-related constants", and if those methods can be moved into the Earth class (as static methods) without creating a dependency from RenderingUtility to Earth. Maybe you need to refactor some of those methods, but it will allow you to place the Earth constants directly where you think they belong.

For example, RenderingUtilities may provide the basic math in form of general coordinate conversion or projection functions, and Earth may reuse these functions to provide earth-specific functions like a Mercator projection. This approach works when all code which requires the Mercator projection has access to the Earth class.

Answer 2

RenderingUtilities as a class probably needs to go. It sounds like it's being used as a catch all for a lot of different things that have better places they could live. If you move things from your static class around it should allow the Earth constants to reside in the Earth class where they belong, and you shouldn't have to get them from a different object or send them to another object.

• Conversions like your example could be extension methods on those types.
• Creating a type for radians and degrees may be useful or maybe creating an angle class that can output its size in the units needed. This could be done to have coordinate or projection types as well.
• The rendering assistance methods could probably live in your RenderObject class.

The reason I recommend creating types is because it should make your core logic easier to understand and code. It is more work initially to have to create types, but it will help move the conversion logic out of your display logic. Another bonus of types is it allows the compiler to catch more errors for you, its easy to accidentally pass degrees to a function that needs radians if they are both simply doubles, naming can help, but it still puts that responsibility on the developer. If instead your function has a radian parameter its much harder to provide the wrong thing, because the compiler will tell you.

Answer 3

OK, you worry about dependencies and you want an OO solution. Let's go all the way then.

I pointed out in a comment that Radius is really a property of Earth and should not be a constant in the first place. I stand by that but it would still require your rendering logic to know about Earth and that would not be very OO. The OO way to fix this is to define an IPlanet interface, have Earth implement that and inject that interface into your rendering logic. Voila!

Earth and rendering logic would both know the interface but would not know about each other.

Answer 4

By having RenderUtilities rely on what is really Earth specific constants, and Earth being a subclass of RenderObject you are sort of implicitly violating the Liskov Substitution Principle, which in short states you should rely on abstractions and not concretions. In your case, this means you could move your Earth-centric operations out of RenderingUtilities and into the Earth class itself, as suggested in Doc Brown's second bulletpoint, putting the relevant operations and constants where they belong.

Alternatively, you could generalize the methods on RenderingUtilities that make use of the Earth constants to instead use RenderingObject getters to retrieve the value you need through inheritance and then inject a RenderObject dependency into the method itself.

Answer 5

The general approach

I believe the constants related to the Earth should be contained in the Earth class

Is there an issue with regards to OOP practices that would prevent me from putting these constants in the Earth class?

The OOP violation here is that you have an Earth class, rather than where you're choosing to put this particular constant. One thing to consider is that in most applications, you wouldn't have an Earth class but rather a Planet class. The Earth is a(n instance of a) planet, and thus it should have the Planet type.

var earth = new Planet()
{
    Name = "Earth",
    Radius = 6371
};

But what's in a (class) name? Well, it makes it clearer that these constant values are not constant to every Planet, but rather to a particular instance of this planet. For example, Mars would have a different radius.

It would be really bad practice if you somehow forced all values for all planets into the Planet class as static constants, since that harshly limits your otherwise freely available instantiation of the Planet class.

However, there are some options here.

1. Setting the planet-specific values can, for example, be put into a factory.

public static class PlanetFactory()
{
    public Planet CreateEarth()
    {
        return new Planet()
        {
            Name = "Earth",
            Radius = 6371
        };
    }

    public Planet CreateMars()
    {
        return new Planet()
        {
            Name = "Mars",
            Radius = 3390
        };
    }
}

From this point on, you can retrieve the values however you like. Maybe the factory does hardcode the values. Or reads them from a file.

2. Rather than adding const values to the class, add a static Planet object.

public class Planet
{
    public string Name { get; set; }
    public int Radius { get; set; }

    public static Planet Earth = new Planet()
                                {
                                    Name = "Earth",
                                    Radius = 6371
                                };

    public static Planet Mars = new Planet()
                                {
                                    Name = "Mars",
                                    Radius = 3390
                                };
}

This way, you are still using non-static properties, but can have static access to a particular Planet instance, e.g. :

var earthCircumference = 2 * Math.PI * Planet.Earth.Radius;

---

If Earth is the only planet

The above answer is a more generalized answer that would apply in most cases, as instance-specific values and class-specific values are two very separate and not interchangeable concepts.

However, if your situation entails that you are only using Earth, never any other planet, then having an Earth class isn't wrong. But at that point, if you consider Earth as a fixed value that never changes or has more than one instance, then you should consider whether the entire Earth class should really be static.

Why would you ever have different Earth instances with some fixed properties (e.g. radius) and some freely choosable (e.g. water)? I don't quite see this working. It's either fixed or not fixed; which means you have two options here:

• Commit to the constant nature of Earth by making the entire Earth class a static collection of the real Earth's properties. Add the constant values to the Earth class but also rework all non-static content of the Earth class to be static.
• Use the non-static Planet class, but somehow provide constant default values to its non-static properties (factory, static Planet property, constructor, default property value, ...)

I would pick the latter option because it seems cleaner and more idiomatic to me, though there are valid use cases for the static approach. It all depends on whether or not you expect to ever want to handle non-default values (for another planet, or a fictional Earth).