Performance improvements

[yusufozturk]

Hi,

Thank you for this project.

I believe some API changes can increase the performance and resource usage. For example, a log file must contain same date layout. Instead of using "Parse" method for each date value, we can detect the locale only one time, and then we can call "ParseInLocation" directly to improve parse process. But right now, locale etc is private fields. So it's not exposed to us, and makes it impossible to change the logic.

If we can make the "LastLocale" field public. after first detection, we can get store the Locale and then call the "ParseInLocation" directly.

Thanks again.

[goodsign]

Hey @yusufozturk

Thanks for the contribution! Won't you want the detectLocale method become public instead? I'm a bit hesitant to make an internal thing such as lastLocale as public, because I believe right now it's just a legacy field which can be removed.

You see, 5 years ago the code was slightly different and the field was needed, because it was called as first line in that method.

And after several changes, right now it can be replaced with just a local variable and even removed at some point, as it's unused. So I'd be hesitant to make it part of API.

But if you have detectLocale as public, you can just use it + ParseInLocation directly.

wdyt?

[yusufozturk]

@goodsign that was my initial idea actually, but "Parse" method has "validateValue", so maybe using that is better?

Also I didn't want to do too many changes, so I thought easier way would be extending the struct.

[goodsign]

@yusufozturk I see your point, but in this case I feel like we'd introduce some hacky API honestly, as the lastLocale shouldn't be a part of the struct in future.

But I'm open to doing this slightly bigger / more properly. If we want a simpler solution and we anyway want to extend the API by one more public thing, maybe we can just do something like:

func (ld *LocaleDetector) DetectLocale(layout, value string) (Locale, error) {
	if ld.validateValue(layout, value) {
		locale := ld.detectLocale(value)
		return locale, nil
	}
	return nil, &time.ParseError{
		Value:   value,
		Layout:  layout,
		Message: fmt.Sprintf("'%s' not matches to '%s' last error position = %d\n", value, layout, ld.lastErrorPosition),
	}
}

And as a bonus — delete the lastLocale field not to confuse future contributors 😆

What do you think?

[goodsign]

The Parse in this case could be transformed into something like:

func (ld *LocaleDetector) Parse(layout, value string) (time.Time, error) {
  locale, err := ld. DetectLocale(layout, value)
  if err != nil {
    return time.Time{}, err
  }
  return ParseInLocation(layout, value, time.UTC, locale)
}

[yusufozturk]

Yes, they look good :) @goodsign
Thanks.

[yusufozturk]

Do you think you can publish a release today? I would like to update our project after that.
Thanks.

[goodsign]

I'd use some help with the PR/tests, actually 😆 As I'm not able at the moment, but I can review a PR with the approach above.