2

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?

  • 7
    There are a few red flags in your question. What's the responsibility of the RenderingUtilities class? Why did you couple your domain object (the Earth class) with your rendering framework? Why does your domain object need to expose its internals (its constants)? Object-oriented design is more than just creating classes, and while I believe it will be next-to-impossible to answer your question without more details, you should know that one of the goals of OO is to encapsulate state within its own class so it doesn't get exposed to the outside world. – Vincent Savard Jan 23 at 19:03
  • 1
    Rendering utilities methods should be set up with parameters instead of using static class variables. The RenderObject::Earth will own those parameters, which it then sends to the RenderingUtilities as parameters. Vincent mentions the backing reasons why! – Patrick Hughes Jan 23 at 19:03
  • "I feel this?" -> That's not a real question. We're not able to determine how you are feeling about something. Can you phrase that a little differently ? My OCD is tickling. – Machado Jan 24 at 11:59
  • @Machado It’s just a title, the question is at the bottom of my post. – PerpetualJ Jan 24 at 13:04
2

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.

  • 1
    Classes that only contain constants are an anti pattern. They end up getting bloated and become the magic string/number class. – Ryathal Jan 24 at 12:58
  • @Ryathal: such classes may not be the perfect solution, but they are at least better than than putting all kinds of constants into an already bloated class like RenderingUtilities. – Doc Brown Jan 24 at 15:05
  • 1
    replacing bad things with bad things isn't really a good option. – Ryathal Jan 24 at 16:39
  • Radius is a property of Earth. A property that does not change is still a property. It only has meaning in the context of earth. – Martin Maat Jan 24 at 22:07
  • @MartinMaat: absolutely, that is why the OP should try to achieve my second solution. But in case it is not easily refactorable to this new structure, there is still my first suggestion as a pragmatic compromise. – Doc Brown Jan 24 at 22:10
0

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.

0

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.

  • Does not convince me. If one wants RenderingLogic to be domain-agnostic, making it depend on something which is called IPlanet (which is clearly a domain term) is not better that making it depend on a class Earth. – Doc Brown Jan 25 at 7:38
  • The rendering logic knows about "grid lines, terrain, water, et cetera". The most generic thing you can offer in this case would be a planet. A sphere would be too generic (has no temperature or atmosphere, stuff that may impact the way you would render the surface). Earth on the other hand would be needlessly specific. Hence IPlanet. The point is you would want to inject something with just enough information for the renderer to do its magic. – Martin Maat Jan 25 at 8:03
0

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.

-1

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.

  1. 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).

  • -1: Introducing a Planet class is grossly unnecessary generalization. – kevin cline Jan 24 at 10:42
  • @kevincline: Did you read the second part of the answer? I didn't say OP must use a planet class; I simply showed the difference between instance-specific and class-specific values, even when both can be considered to be fixed values. – Flater Jan 24 at 10:45
  • Yes. You started out by not answering the OP's question but instead creating and solving an entirely different problem of handling multiple planets where the OP only has one. – kevin cline Jan 26 at 1:49

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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