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

Testing AxisDependency before using the data #843

Closed

Conversation

AndreasIgelCC
Copy link
Contributor

If disabling a YAxis and not setting the datasets AxisDependency correct, there could be strange behaviour (see #665). This change will notify the developer of wrong dataset-settings.

@danielgindi
Copy link
Collaborator

I'm actually not sure that we should pollute the console here.
Imagine adding data on the other axis - but hiding that axis... A common use case, but will produce a warning!
@PhilJay what do you say?

@AndreasIgelCC
Copy link
Contributor Author

Do you have an example in this framework how to show that warning without polluting the console?
Just adding the data on the other axis would be a solution, just removing this data would be a solution too. But in my opinion we should give a feedback if the developer makes a mistake in using the framework (like adding data to disabled axis).

@danielgindi
Copy link
Collaborator

We have no way to know if it's a mistake. Therefore no warning...
The NSLayoutConstraints for example - do issue warnings - when they know that you have absolutely positively made a mistake, due to two colliding constraints.
In our case - what you are describing is completely valid.

@danielgindi
Copy link
Collaborator

It's not even possible (to my knowledge) to do conditional warning like "if you're in debug mode", because those macros get compiled and removed when building the Framework release.

@AndreasIgelCC
Copy link
Contributor Author

Thank you for your feedback! I agree there is a usecase where developer dont want to show axis but want to show data. So always printing a warning would not be a good solution!

Just one question: Print should only write to console when its in debug mode or do I have a misunderstanding in context of frameworks?

@danielgindi
Copy link
Collaborator

I don't really know. From my experience console log works in prouction also. In objc projects I use macros to disable logs in release build. It's possible that it's different in Swift- but still it's too much verbosity...

@pmairoldi
Copy link
Collaborator

Swift's print function works differently. It does not actually print to the device logs like NSLog does. Could precondition be what is needed here?

@danielgindi
Copy link
Collaborator

A precondition would mean that the user is absolutely wrong by having the axis invisible... Which is not true
http://stackoverflow.com/questions/29673027/difference-between-precondition-and-assert-in-swift

@AndreasIgelCC
Copy link
Contributor Author

You are right, the developer might have disabled the axis because he just dont want to see it. Thinking about the topic deeply, I find no condition in which we can say: "The chart-config is invalid!". So you are right in closing the pull request.

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

Successfully merging this pull request may close these issues.

3 participants