Refactoring

[yannicklamprecht]

Needs heavily refactoring

• be SOLID

  • [ ] Single Responsibility Principle
  • [x] Open Closed Principle
  • [x] Liskov Substitution Principle
  • [x] Interface Segregation Principle
  • [ ] Dependency Inversion Principle

• [ ] remove cyclic dependencies ( MsgUtil is depending on Util and the other way around, same for DatabaseHelper)

• [x] properly inject dependencies instead of static abuse shit

• [ ] properly handle configurations

• [x] write Adapter for used functionality in libraries

• [x] use PreparedStatement in the right way

• [x] cache data instead of writing each time -> persist every N minutes

• [x] encapsulate the classes to reduce complexity

• [x] cleanup commands, introduce a subcommand system

• [x] use Player#hasPermission instead of using an explicit PermissionProvider that is already injected into Bukkit system

• [x] remove duplication of ServerNMS and ItemNMS

• [x] refactor MsgUtil

  • reduce code duplication
  • apply SRP

• [x] write javadoc for plugin

• [x] Remove classes that are not used anymore

• [x] refactor shop loader

[Ghost-chu]

use Player#hasPermission instead of using an explicit PermissionProvider that is already injected into Bukkit system Actually this is from upstream https://github.com/KaiKikuchi/QuickShop's code. New code part already use hasPermission,

[yannicklamprecht]

Then it can be removed.

[Ghost-chu]

com.comphenix.packetwrapper maybe will use again in next version, so temporarily not deleted

[Ghost-chu]

cache data instead of writing each time -> persist every N minutes what cache data?

[Ghost-chu]

use PreparedStatement in the right way Actually, i not be good at for SQL...

[Ghost-chu]

properly handle configurations Yes, you are right

[Ghost-chu]

I think i need sleep and find another time to continue refactoring.
00:03 BJT now.

[yannicklamprecht]

cache data instead of writing each time -> persist every N minutes what cache data?

Here we need to evaluate how frequently the database is accessed and if it is necessary to cache certain data changes to reduce load.

Good night.

[Ghost-chu]

Complete Remove classes that are not used anymore

[yannicklamprecht]

What do you think of that config format?

!!org.maxgamer.quickshop.language.Language
command:
  descriptionDebug: '&ePrint debug infomation'
  noTargetGiven: '&cUsage: /qs export mysql|sqlite'
controlpanel:
  empty:
    hover: '&eLooking you want changing shop and click to clear.'
    normal: '&aEmpty: Remove shop all items &e[&d&lOK&e]'
  information: '&aShop Control Panel:'
  modeBuying:
    hover: '&eLooking you want changing shop and click to switch enabled or disabled.'
    normal: '&aShop mode: &bBuying &e[&d&lSwitch&e]'
  modeSelling:
    hover: '&eLooking you want changing shop and click to switch enabled or disabled.'
    normal: '&aShop mode: &bSelling &e[&d&lSwitch&e]'
  price:
    hover: '&eLooking you want changing shop and click to set new price.'
    normal: '&aPrice: &b{0} &e[&d&lSet&e]'
  refill:
    hover: '&eLooking you want changing shop and click to refill.'
    normal: '&aRefill: Refill the shop items &e[&d&lOK&e]'
  remove:
    hover: '&eClick to remove this shop.'
    normal: '&c&l[Remove Shop]'
  setOwner:
    hover: '&eLooking you want changing shop and click to switch owner.'
    normal: '&aOwner: &b{0} &e[&d&lChange&e]'
  sign:
    owner:
      line1: ''
      line2: Enter
      line3: new owner name
      line4: at first line
    price:
      line1: ''
      line2: Enter
      line3: new shop price
      line4: at first line
    refill:
      line1: ''
      line2: Enter amount
      line3: you want fill
      line4: at first line
  unlimited:
    hover: '&eLooking you want changing shop and click to switch enabled or disabled.'
    normal: '&aUnlimited: {0} &e[&d&lSwitch&e]'
language:
  contributors: [
    ]
  country: England
  version: 1
languageVersion: 2
shopDoesNotExist: '&cThere had no shop.'
sign:
  owner:
    line1: ''
    line2: Enter
    line3: new owner name
    line4: at first line
  price:
    line1: ''
    line2: Enter
    line3: new shop price
    line4: at first line
  refill:
    line1: ''
    line2: Enter amount
    line3: you want fill
    line4: at first line
signsUlimited: Unlimited

See Structure: https://github.com/Craftstuebchen/QuickShop-Reremake/tree/remaster/src/main/java/org/maxgamer/quickshop/language

See test case for loading: https://github.com/Craftstuebchen/QuickShop-Reremake/blob/remaster/src/test/java/org/maxgamer/quickshop/language/LanguageProviderTest.java

[Ghost-chu]

404 Not Found

[yannicklamprecht]

404 Not Found

I know did some restructuring after that and forgot to link to a specific revision. https://github.com/Craftstuebchen/QuickShop-Reremake/tree/be3b4efb3cd6f359731de95f611a97003e953df1/src/main/java/org/maxgamer/quickshop/configuration

https://github.com/Craftstuebchen/QuickShop-Reremake/tree/be3b4efb3cd6f359731de95f611a97003e953df1/src/test

[Ghost-chu]

Add a YamlProvider not a good idea in this project. Developer should keep the plugin source is simple and easy to modfiled because this is a Bukkit plugin. If add provider for all configuration, that's will make code mess and waste time.

But you are right, i need improve configuration files.

[yannicklamprecht]

In my opinion the objectification of the configuration is a need.

[Ghost-chu]

This is a how make code clear, simple, and make the configuration file friendly and human-readable question

[Ghost-chu]

So i will create a branch to work(Do a breaking changs on master 100% not a great idea) and try that.

Maybe i can explode the config.yml to mutil files.

[yannicklamprecht]

The Advantage of my system is, that outdated config values are automatically removed, when the model classes doesn't have the value anymore. New attributes are automatically updated if version number is increased in in main model class.

[yannicklamprecht]

I think you should forget everything. The code of this project should be thrown away. Keep the feature description and code it from scratch.

[Ghost-chu]

Issue reopened. I'm cleaning codes and hope it keep the stable.

-> working for adapter configurations

Use reflect to read config to field and write to config from field

[Ghost-chu]

refactored shop loader

[Ghost-chu]

Finished use PreparedStatement in the right way and cache data instead of writing each time -> persist every N minutes

[Ghost-chu]

Finished cleanup commands, introduce a subcommand system in 2.2.0

[Ghost-chu]

This issue can be closed because we're under full rewriting.

[Ghost-chu]

Reopen issue cause v3 under refactoring

[Ghost-chu]

Cleanup the static abuse everywhere. Only use it when really need.

[Ghost-chu]

Refactored MsgUtil class, but still have lots of works need to do.