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

Excessive Printing to Console #417

Closed
pmairoldi opened this issue Sep 24, 2015 · 7 comments
Closed

Excessive Printing to Console #417

pmairoldi opened this issue Sep 24, 2015 · 7 comments

Comments

@pmairoldi
Copy link
Collaborator

I don't know if this is a recent change or what but this line causes a print if the array for y values is empty.

This seems excessive. If the developer when through the trouble of setting a ChartData then they should be aware of what data it holds.

Thoughts?

@danielgindi
Copy link
Collaborator

It's actually there from the first version... But I don't see the problem. It notifies you that something is wrong there. I think maybe a nil check is sufficient.
Problem is mainly with newbies forgetting to add datasets to their data object...

@pmairoldi
Copy link
Collaborator Author

Ya for normal imperative use that is fine but I have created a ReactiveCocoa extension which expects the ChartData to not be nil since it shouldn't really be. Since ChartViewBase has an optional data I need to assign it a value so I just initialize an empty ChartData as can be seen below. This causes the printing to happen. Having only the nil check should be sufficient.

extension ChartViewBase {

    private struct Associated {
        static var data: UInt8 = 0
    }

    public var rac_data: MutableProperty<ChartData> {
        return associatedProperty(self, key: &Associated.data, initial: { self.data ?? ChartData() }, setter: { self.data = $0 })
    }
}

@liuxuan30
Copy link
Member

shall we disable the printing for release mode?

@pmairoldi
Copy link
Collaborator Author

Using print doesn't get printed in the device console like NSLog does so it shouldn't matter.

@liuxuan30
Copy link
Member

alright, main concern is too much print will cause performance drop

@danielgindi
Copy link
Collaborator

This is not Android lol ;-)

@liuxuan30
Copy link
Member

seems we should check the y values. There is a case in #577, x values empty, bar data entries empty, and combined chart continues drawing and crash.

@liuxuan30 liuxuan30 reopened this Nov 28, 2015
liuxuan30 added a commit to liuxuan30/Charts that referenced this issue Nov 30, 2015
…artsOrg#577

We need to check if yValCount is 0, to prevent drawing

This reverts commit 062a61b.
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

No branches or pull requests

3 participants