Classes doing work beyond their scope

[Jeremy-Gaillard]

There's a recurring design issue in the current implementation. A lot of classes do no limit themselves to their role and undertake actions out of their scope. This is very detrimental to the reusability of the classes.

Let's take WMTS_Provider as an example.

The role of the WMTS provider is to implement the WMTS protocol. You give it server information, a tile coordinate, and it gives you an image. Nothing more, nothing less. No assumptions should be made on how the developer will use the class.

Currently, the WMTS provider has:

• Specific functions to retrieve elevation textures and height textures. The provider doesn't care what you'll use the texture for. If you want to fetch textures form different layers, just instantiate two providers.
• Manipulation of tile data. There lies the biggest problem: a class meant to retrieve images from a server is changing the model. Because of this, you can't reuse the class for other purposes than terrain tile creation. The code is harder to understand because the update logic is scattered across multiple classes. The role of the class is no longer clear.

I think the architecture of iTowns as it is now has potential. But the way some classes are implemented makes it really difficult to use and to integrate foreign code into it.

Partial list of classes affected by this problem, followed by the functions that are out of place:

• WMTS_Provider: see above
• WMS_Provider: urlGlobalIR
• Scene: addImageryLayer, addElevationLayer
• TileMesh: use of WMTS coordinates

Debatable:

• Quadtree: up, down, upSubLayer seem out of place. Quadtree's role doesn't seem precisely defined at the moment (BrowseTree #54 has propositions regarding this)

[gchoqueux]

-yes, WMTS_Provider requires work.
-I move addImageryLayer, addElevationLayer.
-TileMesh: use of WMTS coordinates -> I will clean

[Jeremy-Gaillard]

I'm adding ManagerCommands.deQueue to the list.
I don't know exactly how to change it to keep the ability to cancel commands (cancel attribute?), but in the current state, it can cause (already caused in fact) issues that are hard to debug.

[gchoqueux]

Cancel commands is a very important function. It removes all obsolete commands. It avoids a large amount requests.

[Jeremy-Gaillard]

I agree, cancelling commands is important.
But I think the decision whether or not a command should be cancelled must not be done by the managerCommand. Maybe in the clean pass?

[gchoqueux]

the managerCommands manages the commands. This is the best place to decide which command should be execute.

[gchoqueux]

TileMesh: use of WMTS coordinates

I think is removed

WMS_Provider: urlGlobalIR

I think is removed

WMTS

Several thinks are changed can you updated your issue?

[Jeremy-Gaillard]

Seems like all the issues I pointed out were solved. Closing.