Is registry pattern a valid use of global state?

Question

For example, I want to convert json from server to generic object: Shape, as follow:

{
    "type":"Circle",
    "radius":"3",
},
{
    "type":"Rectangle",
    "width":"5",
    "height":"6",
},

using c++ and rapidJson as example here:

/*main.cpp*/
void getResponse(const char* responseText){
    Document d;
    d.parse(responseText);
    for(SizeType i = 0; i < a.Size(); i++){
      if(d[i]["type"]="Circle"){
        this->shapeVector.push_back(new Circle(d[i]));
      }else if(...){
      }
    }

    for(Shape* s : this->shapeVector){
      printf("%f\n",s->getArea());
    }
}

I know it is very bad to use

if(d[i]["type"]="Circle"){
}else if(...){
}

to convert a value to type here, so I try modify it to use registry pattern:

Shape:

/*Shape.h*/
#include <map>
class Shape{
public:
  virtual Shape* jsonToObj(const char* str)=0;
  virtual float getArea()=0; 
  static std::map<const char*,Shape*> objMap;
};
/*Shape.cpp*/
#include "Shape.h"
std::map<const char*,Shape*> Shape::objMap;

Circle:

/*Circle.h*/
#include "Shape.h"
class Circle : public Shape{
public:
  float radius;
  virtual Shape* jsonToObj(Value value){
    Circle* s=new Circle();
    s->radius=value["radius"];
    return s;
  }

  virtual float getArea(){
    return 3.14*this->radius*this->radius;
  }
  static long _;
};
/*Circle.cpp*/
#include "Circle.h"
long Circle::_=[](){ Shape::objMap["Circle"]=new Circle();return 0;}();

main:

/*main.cpp*/
void getResponse(const char* responseText){
    Document d;
    d.parse(responseText);
    for(SizeType i = 0; i < a.Size(); i++){
      this->shapeVector.push_back(Shape::objMap[d[i][name]].jsonToObj(d[i]));
    }

    for(Shape* s : this->shapeVector){
      printf("%f\n",s->getArea());
    }
}

which I convert value "type" to corresponding object type by asking Shape::objMap. When I need to add a type ,for example:Rectangle, I can add Rectangle.h and Rectangle.cpp without modify the if-else in main.cpp:

Rectangle:

/*Rectangle.h*/
#include "Shape.h"
class Rectangle : public Shape{
public:
  float width;
  float height;
  virtual Shape* jsonToObj(Value value){
    Rectangle* s=new Rectangle();
    s->width=value["width"];
    s->height=value["height"];
    return s;
  }

  virtual float getArea(){
    return this->width*this->height;
  }
  static long _;
};
/*Rectangle.cpp*/
#include "Rectangle.h"
long Rectangle::_=[](){ Shape::objMap["Rectangle"]=new Rectangle();return 0;}();

/*main.cpp*/
void getResponse(const char* responseText){
    Document d;
    d.parse(responseText);
    for(SizeType i = 0; i < a.Size(); i++){
      this->shapeVector.push_back(Shape::objMap[d[i][name]].jsonToObj(d[i]));
    }

    for(Shape* s : this->shapeVector){
      printf("%f\n",s->getArea());
    }
}

But according to Why is Global State so Evil?, I should not use global state, which

Shape::objMap

is a global map. My question is, is it a reasonable case to use global state? If-else and global map, which is more "evil" here?

Shouldn't it be a `ShapeFactory` instead of a `Shape`? If `objMap["Circle"]` is a `Circle` then it has a radius, which doesn't make sense. — user253751, Dec 03 '19 at 12:40

Bart van Ingen Schenau · Answer 1 · 2019-12-03T13:04:34.723

The reason why global state is considered evil is because global state often introduces an invisible coupling between different parts of the software. This is especially problematic with mutable global state, because in a program of any size it becomes nearly impossible to track who changed that global state.

Read-only or "append-only" global state is less problematic, because you don't have the problem of unexpected changes coming from everywhere.

With a few small changes, you can make Shape::objMap effectively "append-only" memory in that new shapes can be registered, but existing registrations can't be overwritten.

/*Shape.h*/
#include <map>
class Shape{
public:
  virtual Shape* jsonToObj(const char* str)=0;
  virtual float getArea()=0; 

  static Shape* createShape(const char* name, const char* str)
  {
    if (objMap[name])
      return objMap[name]->jsonToObj(str);
    else
      throw "Requesting unregistered shape";
  }
  static void registerShape(const char* name, Shape* prototype)
  {
    if (objMap[name] == NULL)
        objMap[name] = prototype;
    else
      throw "Shape already registered";
  }
private:
  static std::map<const char*,Shape*> objMap;
};
/*Shape.cpp*/
#include "Shape.h"
std::map<const char*,Shape*> Shape::objMap;

My question is, is it a reasonable case to use global state? If-else and global map, which is more "evil" here?

The registry pattern, especially when implemented such that registry entries can't be overwritten is a reasonable use case for global state. On the other hand, the answer by @VectorZita nicely explains why the if-else ladder isn't evil either.

In the end, it comes down to

Is dynamic registration desirable/required? That isn't possible with the if-else ladder
Do you want compile-time checking that all relevant Shape classes are present? That is not possible with the Registry pattern.
What do you/your team like better?

* If someone accidentally removes Circle.cpp do you want to get an error sooner, or later? — user253751, Dec 03 '19 at 12:51

Vector Zita · Answer 2 · 2019-12-03T10:57:41.070

I know it is very bad to use

if(d[i]["type"]="Circle"){
}else if(...){
}

to convert a value to type here, so I try modify it to use registry pattern:

It is "very bad" to type-check objects that belong to your own hierarchy, which means that you probably have not designed them as flexibly as you could and, even more so, you have not used the "virtual dispatching" mechanisms of the language you are using (abstract classes / interfaces, etc.).

Type-checking "objects" that do not belong to any hierarchy and are, pretty much, only conceptually and conditionally defined upon some general representation (think string and parsing), is practically the only way you have (hence it is NOT very bad) to determine what they must become when entering your own hierarchy. When you are "reading-in" JSON structures (which are simply strings in the end), you are technically working at the "boundaries" of your application (or service, or whatever it is you are building). Applications are not object-oriented at the boundaries.

After objects of your own hierarchy have been constructed, basing almost anything on type-checks is very bad, indeed.

I would suggest that there is no reason for you to retain the original JSON strings in that objMap object, unless you have a good reason to. Your first way is more practical and less convoluted than your second way and for this reason, if I were working with/for you, I would prefer having the first excerpts to maintain, rather than the second ones. Pack them in a class named along the lines of JSONtoShapeConverter or JSONShapeExtractor, so everyone knows that your class parses JSON structures resembling shapes, and returns, well, shapes (as per your type hierarchy) and you will be mostly fine.

In short, I think that your trouble stems not from the global state / registry pattern, but from trying to solve a problem that you don't have.

Is registry pattern a valid use of global state?

2 Answers2