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

KnownTileSources.Create needs refactoring #139

Open
janusw opened this issue Jan 29, 2021 · 1 comment
Open

KnownTileSources.Create needs refactoring #139

janusw opened this issue Jan 29, 2021 · 1 comment

Comments

@janusw
Copy link
Member

janusw commented Jan 29, 2021

This is a follow-up to issue #130 / PR #136, where @pauldendulk commented:

The fact that more an more parameters need to be added indicates something else is lacking in the design. Since there is no better solution let's just add the extra parameters.

A better solution might be to make all the getters settable. Or perhaps work with immutable classes and add methods to create mutated copies (the functional way). One practical reason to choose the first option is that I would like to schema to be serializable. Actually this is an important feature I would like to add. We had some way to serialize once but it had a lot of custom code. I would like the standard tools to serialize the class as it is.

In #130, @FObermaier commented:

IMHO we should remove KnownTileSources and replace it with some configuration file that can be altered locally according to specific needs. I think @bertt had/has sth like this for ArcBruTile.

@pauldendulk
Copy link
Contributor

A cleaner solution would be to remove all known tile sources, but my impression is that the majority of users use the KnownTileSources. It also helps a lot in getting started, so I do not intend to remove it. At least not soon. I may introduce a service for known tile sources, alongside KnownTileSources.

About the refactoring. My purpose is to do ordinary json (de)serialization on the tile schemas. My experience with newtonsoft json is that you need to add public getters and setters. I am okay with that, even though this breaks encapsulation. There may be other solutions though, like other libraries or write our own serialization with reflection. My preference at the moment (but I frequently change my mind) is to just add setters. The TileSchema should be simple data thing. Perhaps some of the methods should be move out of it.

My strategy right now would be to create a feature branch and start to write unit tests for serialization and deserialization. First for the simplest TileSchemas, then the others. There may be complication that require a serious rewrite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants