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

Replace ILayer.Style with a ILayer.Styles like IFeature.Styles #2452

Open
pauldendulk opened this issue Jan 20, 2024 · 2 comments
Open

Replace ILayer.Style with a ILayer.Styles like IFeature.Styles #2452

pauldendulk opened this issue Jan 20, 2024 · 2 comments
Milestone

Comments

@pauldendulk
Copy link
Member

pauldendulk commented Jan 20, 2024

The problem

  • The ILayer.Style is a single item while the IFeature.Styles is a collection. That is odd.
  • The default layer style is a common confusion for users that work with styles on features.
  • Sometimes users want one style to be on top of another (like a callout that needs to be above all the symbols). This is now possible by adding the symbol style and callout style to a StyleCollection and assign it to the ILayer.Style. You have to know about this possibility something. If we have a ILayer.Styles users will be able to discover that functionality.

Complications

  • If there is no default symbol users will not see anything in their map unless they explicitly add it. One solution would be to just leave this up to the user, but perhaps this will result in a lot of frustration. Solutions:
    • Perhaps we should write a warning to the log, but then again sometimes the users just does not want a symbol (yes there will be cases where users want this), should we write a warning?
    • Render something in a gray semi-transparent default style as long as there is no style. The user will at least see something. Perhaps only in debug mode. Problem: but our design is that the style determines the renderer, but here we would need some workaround.
    • Add a default symbol to the ILayer.Styles anyway. So, the layer will just have a single style when created. Downside: There will still be confusion for people working with IFeature.Styles. We have to pick some default style for each layer.
  • The current RasterFeature needs a RasterStyle. This is necessary in our logic because the style determines the renderer. It makes no sense for the user to add this, it adds no information, it is just necessary to make it work. Currently we solve this by setting a default style. This solution will disappear, so we have to come up with an alternative.
  • Many samples would need extra lines to add the style. It would be nice of the minimal code would be necessary to add a layer.

The proposed solution

  • Replace the ILayer.Style with an ILayer.Styles collection like in the IFeature.Styles.
  • The ILayer.Styles will be empty by default.
  • Create the OpenStreetMap layer with an extension method which adds the RasterStyle
  • IMPORTANT: Add a warning to the log whenever a feature has not style in ILayer.Styles and IFeature.Styles. Make this depend on a Map.LogNoStyleWarning property so that it is possible to remove this warning if you choose to have no styles in some scenarios. In the log message mention that Map.LogNoStyleWarning = false can be used to turn that message off.
@charlenni
Copy link
Member

One thing first: the layer style is drawn first, then the feature style.

The problem comes up only for new users. They need some time to get, what a feature and what a style is. They want a fast success by seeing the map and a feature on the screen. We will have such thing when bringing the MapView things to MapControl. one thing is the Marker. There is no need to think about features and styles. If they want to have more advanced styles, then they have to go deeper into features and style. The concept is very flexible, but you need some time to get it.

As first step I would things more consistent by using same names and same types.

@pauldendulk
Copy link
Member Author

The problem comes up only for new users. They need some time to get, what a feature and what a style is. They want a fast success by seeing the map and a feature on the screen. We will have such thing when bringing the MapView things to MapControl. one thing is the Marker. There is no need to think about features and styles. If they want to have more advanced styles, then they have to go deeper into features and style. The concept is very flexible, but you need some time to get it.

This is the 'ease of use' argument, which is a valid argument, but you have to carefully weigh the pros and cons every time. There are three things I want to say about it.

Ease of use is often a short term thing

Very often a solution that improves 'ease of use' at first only complicates things later on. I see this very often in api designs where the implementation is abstracted away from the user. The user is told to just change this little thing and then everything works, they don't have to understand what goes on underneath the hood. But where there is something failing underneath or they need to support more complicated scenarios it becomes harder to fix. There are tweaks to get things done in their current solution, but it is hard to get right because users still don't understand what is going on. To get back on track again they have to switch to another kind of solution, but users often stay stuck in their current solution because the switch seems such a big investment.

Also the component developers go along with the users on this 'ease of use' track, by adding more add-hoc functionality to their 'ease of use' solutions, which keeps users longer on this dead end road.

An example is our editing support. This was originally part of the samples. So, low ease of use. Users would have to copy a lot of code to get it working. Then we moved this to the widgets (in itself a good decision because it makes things more maintainable), which made it very easy to get started, but then it did not support all user scenarios and we received feature requests. We could go along with this by making the editing widget more flexibly, but it ends somewhere. So, I think that by pointing users to the editing widget implementation we can prevent a lot of misery.

Add ease of use by offering composition

Above we described a problem of many 'ease of use' solutions, but I think it does not have to be that way. With the Map extension method approach I hope to achieve the same ease, while also putting the user on the compositional track. The extension methods make it easy to get started, but we should immediately point them to the implementation of the extension method and should stimulate them to copy that code to their own solution as soon as they require more advanced functionality. In general I think it is good to point users to our implementation. I have seen you (@charlenni) answer a question by posting the url to the VisibleFeatureIterator.cs. I like it that way. And we should try to keep our code comprehensible to our users.

About ease of use with composition. Instead of this:

Map.Widgets.Add(new SomeComplicatedWidget());

I would prefer this:

Map.AddSomeComplicatedWidget();

That is just as easy. Not sure it is possible to make it this easy, but this is what I aim for. We may also find other ways to support composition.

Ease of use versus maintenance

Adding support to make specific scenarios easy often complicates the code. And, like described above, when you start on that track you later have to add more options to support other scenarios. Also, making general changes becomes harder because you need to take into account the effect on those components. More stuff means more problems, which means less time to make progress on more fundamental changes. I am maintaining this project now for ten years and I intend to continue doing that. So, I feel free to choose not to add functionality if I suspect it will make the maintenance higher.

@pauldendulk pauldendulk added this to the 5.0 milestone Mar 17, 2024
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