vendredi 29 septembre 2017

Adding an extra behavior to old code

I need a design input on a programming problem I'm trying to solve.

The problem:

I have old Licensing class and a new Licensing class. I have an app that uses the old Licensing class. The app has calls of the old Licensing class methods everywhere in the code.

After looking at the old Licensing closely I realized that the only thing separating it from the new Licensing class what kind of file it reads. Old: Reads .ini files (normal textfile), New: reads .file (binary).The new licensing tool reads a binary file only it can understand. But I know that if there is a way I make the old Licensing class to understand .file then it saves me from going everywhere in the app and doing things like if (License::Instance().isFeature1Licensed() || NewLicensingTool::Instance.isFeature1Licensed()).

Requirement:

  • App must accept both types of file for licensing (.ini) or (.file)
  • Old Licensing tool must not know anything about new licensing tool or vice versa

Here is the current general layout of the app:

// Singleton
class License
{
public:

.
.
.

   void read (string& filename)
   {
    // read a specific kind (.ini) of file
    // a certain way
   }
.
.
.   
}


// This exists in the app level
// This class requires a lot of
// dependency
class NewLicensingTool
{
public:

.
.
.

   void read (string& filename)
   {
    // reads a specific kind of file (.file)
    // a different way than the old way
   }

   vector<NewLicensingTool::LicensingIDs> getLicenses()

   .
   .
   .
   }


class App
{
  // needs to be able to license the old and
  // new way
  // There are calls to License::Instance() methods
  // everywhere in the code
...
void doSomething()
{
    ...
    if (License::Instance().isFeature1Licensed())
    {
        ...
    }
    ...
}

...

void doAnother()
{
    ..
    if (License::Instance().isFeature2Licensed())
    {

    }
    ..
}
}

Here is my proposed solution:

class OtherReadBehavior
{
public:
   void read(string& filename) = 0;
}

// Singleton
class License
{
public:

.
.
.

   void read (string& filename)
   {
    // read a specific kind (.ini) of file
    // a certain way

    OtherReadBehavior::read(filename);
   }
.
.
.   
}


// This exists in the app level
// This class requires a lot of
// dependency
class NewLicensingTool
{
public:

.
.
.

   void read (string& filename)
   {
    // reads a specific kind of file (.file)
    // a different way than the old way
   }

   vector<NewLicensingTool::LicensingIDs> getLicenses()

   .
   .
   .
   }


class App
{
  // needs to be able to license the old and
  // new way
  // There are calls to License::Instance() methods
  // everywhere in the code
class NewLicensingReadBehavior : public OtherReadBehavior
{
public:
   void read(string& filename)
   {
      NewLicensingTool::Instance().read(filename);
   }
}
...
void doSomething()
{
    ...
    if (License::Instance().isFeature1Licensed())
    {
        ...
    }
    ...
}

...

void doAnother()
{
    ..
    if (License::Instance().isFeature2Licensed())
    {

    }
    ..
}
}

I'm not sure if this is a good design or an ugly design. Before I present it to my peers, I want to get experts opinion on this. And if you know a better way to do it or pattern I should look up. Please let me know.

update

I just realize this solution won't work because the old License class has a private array of the valid Licenses. So even tho NewLicensingBehavior inherits from OtherLicensingBehavior, it will not be able to set the valid licenses array.

Aucun commentaire:

Enregistrer un commentaire