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

PieChartData initialisers ambiguous when xVals parameter is nil #224

Closed
toriaezunama opened this issue Jul 16, 2015 · 4 comments
Closed

Comments

@toriaezunama
Copy link

Not sure if this is a bug but it makes for messy API calls like: PieChartData(xVals: nil as [String?]?, dataSet: dataSet)

The following API pairs are ambiguous when xVals is set to nil because both are valid so the compiler doesn't know which to choose.

PieChartData(xVals: [NSObject]?)
PieChartData(xVals: [String]?)

PieChartData(xVals: [NSObject]?, dataSet: ChartDataSet?)
PieChartData(xVals: [String]?, dataSet: ChartDataSet?)

PieChartData(xVals: [NSObject]?, dataSets: [ChartDataSet]?)
PieChartData(xVals: [String]?, dataSets: [ChartDataSet]?)
@danielgindi
Copy link
Collaborator

Hmmm... This is an interesting problem!

In one hand, we have a variable of kind [NSObject]? or [String]? which may point to nil, and we should be able to pass it in.
On the other hand, you have the ugly PieChartData(xVals: nil as [String?]?, dataSet: dataSet).
This problem exists in other languages as well (i.e. Java...)

But theoretically it could be solved if we would be able to hint the compiler that "it doesn't matter which constructor you choose" or "THAT one is the right constructor when there's ambiguity".
Unfortunately I do not know of such a declaration.

I'm going to try Apple forums for this, who know maybe there is a way...

@danielgindi
Copy link
Collaborator

It just hit me no the head. We need to just add a constructor that accepts no arguments.
But in this case, you want to pass a dataset, without xIndexes, which is an unusual case. I'm guessing you are using Combined chart.

So I could have added a constructor which takes only dataSet, but that would present a documentation problem. Users will start trying to just initialize a chart without xIndexes, and not figuring out why it is not working.

I think it is preferred to have it left as it is, to make things clear. On occasions like this it will not be the prettiest it can be, but it's not that bad!
It's a good compromise, I think.

@toriaezunama
Copy link
Author

My use case was a piechart without any labels (because I want to show them in a different way to the built in way).

One option is to be explicit in the parameter naming e.g

    public init(xStringVals: [String?]?, dataSets: [ChartDataSet]?)
    public init(xNSObjectVals: [NSObject]?, dataSets: [ChartDataSet]?) 

But this messes up the nice uniform API.

Another option is to let the compiler decide.

    public init(_ xVals: [String?]?, dataSets: [ChartDataSet]?)
    public init(_ xVals: [NSObject]?, dataSets: [ChartDataSet]?) 

But now you lose the explicit xVals and the call looks like: PieChartData(nil, dataSets: [])

Or, define constants to use when there is no 1st parameter.

let kNilNSObjects = Optional<[NSObject]>.None
let kNilStrings = Optional<[String?]>.None

But the user of the API needs know about the cause of the ambiguity when the compiler gives the error and they need to know about this typealias.

Lastly, you could maybe just explain this situation in the docs.

Designing an API is hard!

Thanks for all the work you have put in on this great library.

@danielgindi
Copy link
Collaborator

You are missing something:
You have to have x Values in the chart. You don't have to draw them, but you do have to have them.

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

2 participants