Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/economy #3663

Merged
merged 28 commits into from Jan 29, 2020
Merged

Refactor/economy #3663

merged 28 commits into from Jan 29, 2020

Conversation

@suneettipirneni
Copy link
Contributor

suneettipirneni commented Jan 24, 2020

Description:

Motivation

The motivation may seem a bit elusive at first, as detailed explanation is required. We should kick off this explanation with a review of the form of objects rather than the traditional perspective of function.

Let's see an example.

I'm going to use the route of inheritance via objects to demonstrate an issue.
Observe the following code:

public class Animal {
    ...
}
public class AnimalDiet extends Animal {
    ...
}
// A Panda has a Diet
public class Panda extends AnimalDiet {
    ...
}
// A Bear has a Diet
public class Bear extends AnimalDiet {
    
}

So for the code above fits our needs, now let's introduce a catalyst for failure:

public class CarnivorousAnimal extends Animal {
    ...
}

But now I need my Bear class to extend CarnivorousAnimal because it is also a carnivore and has a diet. Now I'm going to have to use an inheritance chain:

public class CarnivorousAnimal extends AnimalDiet {
    ...
}

public class Bear extends CarnivorousAnimal {
    ...
}

We now have the problem where we've locked CarnivorousAnimal into objects that only have both a diet and are carnivores. This is where this kind of design breaks down, it's not future proof, and it's highly static as you need to change superclasses in order to make changes in subclasses. So what's the solution to this?

There's actually two:

  1. Create an interface for each attribute (Good if there is variation between classes)
  2. Instead of using class inheritance make the attributes of the class in the form of a field. (Good if there isn't much variation in implementation)

So let's fix the code, (I'm going to use a mix of method 1 and 2 for this):

public class Animal {
    ...
}
public class Diet {
    ...
}
public interface Dietable {
    Diet getDiet();
}
public interface Carnivorous {
    ...
}
public class Bear extends Animal implements Carnivorous, Dietable {
    private Diet diet;
    
    @Override 
    public Diet getDiet() {
        return diet;
    }
}

Notice how now the clarity of the object inheritance has been restored whilst at the same time allowing for future changes to be easier.

This same idea has been applied to TownyEconomyObject, for the same or similar reasons as outlined above. In addition, new interfaces have been introduced to further declutter a rather messy inheritance chain.

New Interfaces:

  • TownBlockOwner - Now applied to Resident and Town to generify uses.
  • EconomyHandler - Used to show that an object can handle economic interactions.
  • Permissible - Used to show that an object can handle permission interactions.

With the addition of these interfaces the main Towny objects now directly inherit from TownyObject providing a clear and concise object hierarchy that is not only aesthetically pleasing but also modular enough for future expansion.


  • I have tested this pull request for defects on a server. (Reccomended that others test as well)

By making this pull request, I represent that I have the right to waive copyright and related rights to my contribution, and agree that all copyright and related rights in my contributions are waived, and I acknowledge that the TownyAdvanced organization has the copyright to use and modify my contribution under the Towny License for perpetuity.

Just messing with some ideas, have static EconomyManager and an interface. The latter is used for class specific items such as econ name, and the other is used for function that are class agnostic.
Made it so the account field is now implemented in most areas in towny.
Added two new interfaces
- Permissible
- TownBlockOwner (A port from the original class)

Since both of these interfaces are implemented, Resident and Town now both directly inherit from TownyObject.
@suneettipirneni suneettipirneni requested a review from LlmDl Jan 24, 2020
@suneettipirneni suneettipirneni requested a review from LlmDl Jan 24, 2020
@suneettipirneni

This comment has been minimized.

Copy link
Contributor Author

suneettipirneni commented Jan 25, 2020

Should be good now

@LlmDl LlmDl merged commit 734a3f6 into master Jan 29, 2020
LlmDl added a commit that referenced this pull request Jan 29, 2020
  - Eco backend refactor courtesy of Siris with PR #3663.
@suneettipirneni suneettipirneni deleted the refactor/economy branch Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.