Using minimum of 0 as default

[simonnickel]

After I updated to the current version I got really confused with a BarChart having a minimum value above 0. In the earlier version 0 was used as the base value for bar height calculation. In the current version the lowest number is used. (Bar with 10, 20, 40 on 40px is shown with heights of 0px, 10px, 40px - not my expected 10px, 20px, 40px). It took some time until I figured out why this happened.

However, I think using 0 as default minimum value is what you would expect using a JBBarChart and therefore should be the default setting.

Btw: great tool and keep on with constant improvements!

[hackingotter]

Setting the minimum value to zero as default would remove the ability to create a diagram with normalized bars.

I think we should create a property to switch between normalized and non-normalized and use non-normalized as default. This would solve the confusion and preserve the feature to create normalized charts.

[terryworona]

@simonnickel would setting the chart's minimumValue to 0 not solve this for you?

@hackingotter the chart would still be normalized, with 0 being used as the internal minimum value, instead of 10 (as in @simonnickel's example).

[simonnickel]

@terryworona Yes, that was the solution. But I did not expect that I have to set it. An additional setting like useMinimumValueAsHeightBase (just with a better name) with NO as default would help to make it clear. Like I said: I would not expect the bar to cut my values as default behavior.

[hackingotter]

@terryworona Maybe I missunderstand something but when you set minimumValue in JBChartView to 0 as default - (CGFloat)mininumValue always will return the the minimumValue (not normalized) instead of cachedMinHeight (normalized) because minimumValue doesn't equal kJBChartViewUndefinedMinimumValue anymore. If you now set minimumHeight to kJBChartViewUndefinedMinimumValue you get an alert as its value is negative.

@simonnickel I share your opinion as I was confused by this, too. I think this would make things much more clear.

[terryworona]

@hackingotter yes, you are correct; very subtle bug.

I do prefer to expose the minimum and maximum values in case someone wants to add padding (especially useful for line charts).

[terryworona]

Alright, made some changes:

• minimumValue and maximumValue still exist
• default minimumValue is now 0
• default maxmimumValue is the datasource's max.
• To override, simply set the min or max. To reset, call the appropriate resetter: [self resetMinimumValue] or [self resetMaxmimumValue].

The only gotcha with the pattern is the assumption that the y-axis starts at 0 (minimumValue default = 0). A very quick look at the documentation should provide some insight though.

[terryworona]

Change is live in v2.5.0 commit: b59904e

[hackingotter]

I think there is an error in your code.

Its about _hasMinimumValue:

The chart will be normalized if _hasMinimumValue is NO, right?
But _hasMinimumValue cannot be set to NO.

I would suggest to create a method -(void)normalizeValues:(BOOL)normalize which sets this value.

[terryworona]

The minimumValue and maximumValue accessors in both the bar and line chart views are only utilized when normalizing a height via:

- (CGFloat)normalizedHeightForRawHeight:(NSNumber*)rawHeight

Regardless if hasMinimumValue returns true or false, the returned value from minimumValue will be normalized regardless.

Perhaps I am missing something? Maybe take me through the code path your thinking?

[hackingotter]

hasMinimumValue always is YES, as there is no method in JBChartView could set it to NO, only one that sets it to YES.

Then I looked at JBLineChartView:

- (CGFloat)minimumValue
{
    if ([self hasMinimumValue])
    {
        return fminf(self.cachedMinHeight, [super minimumValue]);
    }
    return self.cachedMinHeight;
}

It will alway return fminf(self.cachedMinHeight, [super minimumValue]) as the if statement is always fullfilled.

So if you call resetMinimumValue as you don't want the bars to be cut the minimumValue will be 0 and hasMinimumValue will be YES.
So -(CGFloat)minimumValue will return 0 as hasMinimumValue is true and 0 is smaller than the cachedMinHeight.

And this is the error. At this point the cachedMinHeight should be returned to cut the bars and not 0.

To solve this hasMinimumValue somhow has to be set to NO.

That's how I read the code. Maybe I'm wrong. This really isn't as easy as it seems.

And maybe I also missunderstand "normalized". Does it include cutting the bars or does it only mean, that the bars are converted to be in the right relation to each other?

[terryworona]

Normalization is just taking a series of data and having it speak a common language; in our case, it's adjusting their heights to be relative the chart's bounds (height).

hasMinimumValue will in fact always return YES, but that is because we chose to make the default 0.

If we chose to NOT make the default minimum 0, then we can remove this piece and have it return the chart's actual minimum value.

What exactly in the current implementation breaking for you?

[hackingotter]

Now I would like the bars to be cut so the difference between them can be seen better. In earlier versions this would have been the default behaviour but now it is not possible anymore.

As I explained before the reason for this is, that hasMinimumValue is always YES.

The 3rd paragraph i don't understand.

I would solve the problem simply by adding a method like -(void)cutBars:(BOOL)cut. If cut == YES then hasMinimumValue is set to NO, if cut == NO, hasMinimumValue is set to YES.

And if resetMinimumValue is called this will be reseted, too.

[terryworona]

Ah I see what you mean.

By defaulting the minimumValue to 0.0, you can never tell the chart to use the actual minimum of the dataSource.

By automatically defaulting to 0.0 for the minimum, I was address @simonnickel's concern.

I think it's best to default to the chart's min and max values, and noting in the readme and documentation that the minimum value can be manually set to 0 if that's what you need.

[terryworona]

@hackingotter fixed pushed to v2.5.1 commit: 49206a0

@hackingotter for your case, I would try setting the minimumValue = smallest chart value - 10. Similarly, to add top padding maximumValue = largest chart value + 10.

[hackingotter]

Thanks but this was just an example I used to show you what I meant.

Now everything should work again.