created refactor doc

[TheseusGrey]

this doc outlines the solutions we've come up with to solve the circular dependency issues within our projects.

---

This change is 

[OlafSzmidt]

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dent50cent)

a discussion (no related file):
Not convinced that this should be in the docs if you're describing possible solutions to an issue in the code. Seems more like this needs an issue.

Also, if you're going to throw in files into the folder like that, make sure they're linked properly in the root README.md etc. There's a folder pattern and each has a README as a TOC.

---

[TheseusGrey]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dent50cent)

a discussion (no related file):

Previously, OlafSzmidt (Olaf Szmidt) wrote…

Not convinced that this should be in the docs if you're describing possible solutions to an issue in the code. Seems more like this needs an issue.

Also, if you're going to throw in files into the folder like that, make sure they're linked properly in the root README.md etc. There's a folder pattern and each has a README as a TOC.

The reason this is in a doc instead of an issue was for the benefit of James, as we need to discuss these solutions with him before creating issues to act on them. The doc is just to describe what we discussed in the technical chapter. As for why it's not linked, it's not a final document and is not intended to be a permanent part of the documentation, i was also instructed by the team leader to make this pull request so other members of the team to review the document to make changes or corrections before we come to a final decision.

---

[OlafSzmidt]

Reviewable status: complete! all files reviewed, all discussions resolved

a discussion (no related file):

Previously, dent50cent wrote…

The reason this is in a doc instead of an issue was for the benefit of James, as we need to discuss these solutions with him before creating issues to act on them. The doc is just to describe what we discussed in the technical chapter. As for why it's not linked, it's not a final document and is not intended to be a permanent part of the documentation, i was also instructed by the team leader to make this pull request so other members of the team to review the document to make changes or corrections before we come to a final decision.

You describe the issue (ie. Projects are too coupled leading to circ. dep) and then you discuss your views in the comments, no? :p Seems odd creating a doc to discuss options, but resolving if that's how you want to do it

---

[TheseusGrey]

Reviewable status: complete! all files reviewed, all discussions resolved

a discussion (no related file):

Previously, OlafSzmidt (Olaf Szmidt) wrote…

You describe the issue (ie. Projects are too coupled leading to circ. dep) and then you discuss your views in the comments, no? :p Seems odd creating a doc to discuss options, but resolving if that's how you want to do it

i don't get what you mean, in the doc i explain the problem, why it should be solved. Then discusses the different options, how we might go about them, and some of the consequences of each option. I don't resolve anything in this document.

---

[OlafSzmidt]

Reviewable status: complete! all files reviewed, all discussions resolved

a discussion (no related file):

Previously, dent50cent wrote…

i don't get what you mean, in the doc i explain the problem, why it should be solved. Then discusses the different options, how we might go about them, and some of the consequences of each option. I don't resolve anything in this document.

Blocking discussions need to be resolved on reviewable, if I don't resolve them you can't merge... That's all I meant. Jesus 😂😂😂

---

[j4mesholland]

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @dent50cent)

---

docs/CFL-refactor.md, line 2 at r1 (raw file):

Currently upgrading versions for our different projects is far more effort than it needs to be. So it has been discussed by members of the team that a refactor might be worth looking into.

Currently, upgrading our different projects is far more effort than it needs to be. So it has been discussed by members of the team that a refactor might be worth looking into.

---

docs/CFL-refactor.md, line 13 at r1 (raw file):

Currently there is a circular dependency between Rapid-Router and the Portal, (see above diagram). This has lead to code that is messy in places, and has made upgrading package versions incredibly difficult. This has been an issue for a number of years it seems, with mentions to it [here](https://github.com/ocadotechnology/rapid-router/issues/688) and [here](https://github.com/ocadotechnology/rapid-router/issues/658) dating back to 2015.
 
While this is a long and potentially challenging task, i feel it would make long-term development and maintenance of the various repositories significantly easier.

There is a circular dependency between Rapid-Router and the Portal, (see above diagram). This has lead to code that is messy in places and has made upgrading package versions difficult. This has been an issue for a few years, dating back to 2015. See past issues: here and here.

Whilst this is a challenging task, it would make long-term development and maintenance of the various repositories significantly easier.

---

docs/CFL-refactor.md, line 17 at r1 (raw file):

While this is a long and potentially challenging task, i feel it would make long term development and maintenance of the various repositories significantly easier. 

## Solutions

## Solution ideas

---

docs/CFL-refactor.md, line 19 at r1 (raw file):

## Solutions

### Refactor to new model

Option 1: Refactor to new model

---

docs/CFL-refactor.md, line 29 at r1 (raw file):

The above diagram shows a new model with which to organise our current projects. This model should eliminate the currently existing circular dependancy and should prevent it from occurring in future. The main idea involves separating the models section from portal, and placing it within it's own repository if needed, (written as "CFLmodel" within this document). this CFLmodel would contain all the things the models currently do within the portal repository, this includes: Storing and retrieving data, validating data, relations between the data, and how the data can behave.·

Upper case T on This CFL model...

---

docs/CFL-refactor.md, line 35 at r1 (raw file):

With the setup, updating package versions becomes a much simpler process. As first we need to upgrade our models to the desired version, then we can upgrade AI:MMO and/or Rapid-Router, and finally once all of that has been upgraded we can upgrade Portal.

With the setup, updating package versions would become a much simpler process. First, we would need to upgrade our models to the desired version, then we would be able to upgrade AI:MMO and/or Rapid-Router, and finally once that has been upgraded we can upgrade Portal.

---

docs/CFL-refactor.md, line 37 at r1 (raw file):

In conclusion, this approach would require potentially a lot of refactoring of our codebases. However most of the work should just involve changing where the imports come from, and creating this need CFLmodel repository.

It might be better to list some pros and cons here:

Pros:

• Easier to understand the structure for a new contributor
• Clearer legacy code
• Easier to debug

Cons:

• May take a long time to complete
• More work

---

docs/CFL-refactor.md, line 40 at r1 (raw file):

### Dependancy injections

Option 2: Dependancy injections

---

docs/CFL-refactor.md, line 41 at r1 (raw file):

This is the other proposed solution. it has 3 key concepts, which are as follows:

This option has 3 key concepts, which are as follows:

---

docs/CFL-refactor.md, line 48 at r1 (raw file):

These are taken from [this](https://wiki.python.org/moin/DependencyInjectionPattern) python wiki page, i've also linked to a page explaining the idea in more detail [here](http://code.activestate.com/recipes/413268/).

The advantage of this approach over the previous one, is that this will not require as significant a refactor of our code, I have also been told there is an existing example of this within AI:MMO which we can use as reference.

It might be better to list some pros and cons here:

Pros:

• Faster to implement

Cons:

• Doesn't fix legacy code issues
• Harder to debug

[faucomte97]

Reviewable status: all files reviewed, 18 unresolved discussions (waiting on @dent50cent)

---

docs/CFL-refactor.md, line 13 at r1 (raw file):

Currently there is a circular dependency between Rapid-Router and the Portal, (see above diagram). This has lead to code that is messy in places, and has made upgrading package versions incredibly difficult. This has been an issue for a number of years it seems, with mentions to it here and here dating back to 2015.

Past tense of 'lead': this has led to code

---

docs/CFL-refactor.md, line 15 at r1 (raw file):

Currently there is a circular dependency between Rapid-Router and the Portal, (see above diagram). This has lead to code that is messy in places, and has made upgrading package versions incredibly difficult. This has been an issue for a number of years it seems, with mentions to it [here](https://github.com/ocadotechnology/rapid-router/issues/688) and [here](https://github.com/ocadotechnology/rapid-router/issues/658) dating back to 2015.

While this is a long and potentially challenging task, i feel it would make long term development and maintenance of the various repositories significantly easier. 

Capital I: I feel like

---

docs/CFL-refactor.md, line 29 at r1 (raw file):

RapidRouter-->Portal;

The above diagram shows a new model with which to organise our current projects. This model should eliminate the currently existing circular dependancy and should prevent it from occurring in future. The main idea involves separating the models section from portal, and placing it within it's own repository if needed, (written as "CFLmodel" within this document). this CFLmodel would contain all the things the models currently do within the portal repository, this includes: Storing and retrieving data, validating data, relations between the data, and how the data can behave.

Placing it within its own repository (possessive).

---

docs/CFL-refactor.md, line 37 at r1 (raw file):

With the setup, updating package versions becomes a much simpler process. As first we need to upgrade our models to the desired version, then we can upgrade AI:MMO and/or Rapid-Router, and finally once all of that has been upgraded we can upgrade Portal.

In conclusion, this approach would require potentially a lot of refactoring of our codebases. However most of the work should just involve changing where the imports come from, and creating this need CFLmodel repository.

'Creating this need CFLmodel repository' ==> needed?

---

docs/CFL-refactor.md, line 43 at r1 (raw file):

This is the other proposed solution. it has 3 key concepts, which are as follows:
 * Components do not know each other directly.
 * Components specify external dependencies using some kind of a key.

Dependancies ;P

---

docs/CFL-refactor.md, line 44 at r1 (raw file):

 * Components do not know each other directly.
 * Components specify external dependencies using some kind of a key.
 * Some "superior instance" (the IoC container, for example) resolves the dependencies once for each component and hereby "wires" the components together.

Dependancies

---

docs/CFL-refactor.md, line 46 at r1 (raw file):

 * Some "superior instance" (the IoC container, for example) resolves the dependencies once for each component and hereby "wires" the components together.

These are taken from [this](https://wiki.python.org/moin/DependencyInjectionPattern) python wiki page, i've also linked to a page explaining the idea in more detail [here](http://code.activestate.com/recipes/413268/).

Capital I (I've)

---

docs/CFL-refactor.md, line 48 at r1 (raw file):

These are taken from [this](https://wiki.python.org/moin/DependencyInjectionPattern) python wiki page, i've also linked to a page explaining the idea in more detail [here](http://code.activestate.com/recipes/413268/).

The advantage of this approach over the previous one, is that this will not require as significant a refactor of our code, I have also been told there is an existing example of this within AI:MMO which we can use as reference.

Remove comma between 'previous one' and 'is that'.
Replace comma after 'of our code' with a full stop (or at least a semi-colon).

[faucomte97]

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @dent50cent)

[TheseusGrey]

Document does not need to be merged in, and is no longer needed