Skip to content

Simplify titlecase() to unique + match (drop mutable cache) - #164

Merged
LittleBeannie merged 1 commit into
mainfrom
simplify-titlecase
Sep 24, 2026
Merged

LittleBeannie merged 1 commit into
mainfrom
simplify-titlecase

Conversation

@yihui

@yihui yihui commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

What

titlecase() previously used a local({}) closure with a mutable cache (list keyed by input value) to memoize tools::toTitleCase() calls across invocations (issue #129). This replaces it with a plain unique + match:

convert <- function(text) {
  if (lower) text <- tolower(text)
  u <- unique(text)
  tools::toTitleCase(u)[match(text, u)]
}

Why

  • The mutable cache added state and complexity for no benefit. toTitleCase() was already only ever called on distinct values within a single call; the cross-call cache only helped if the same strings recurred across separate titlecase() invocations, which is not the hot path.
  • Benchmarked over 20 runs on 200k values: the simple version (1.95s) is faster than the warm-cache memoized version (2.26s), because building/reading the cache list costs more than it saves.
  • Output is byte-identical: verified on character vectors, factors (levels preserved), empty input, and the existing lower = TRUE/FALSE paths.

Testing

  • tests/testthat/test-ae_listing.R (covers titlecase()/propercase()): 5/5 pass.
  • Full suite: 0 failures, 21 tests.

This PR is independent of the bug-fix PR (#162) and the vectorization PR — it touches only R/ae_listing.R.

titlecase() used a local() closure holding two lists that were grown on
every cache miss and lived for the whole session. The speedup it was
meant to provide (#129) comes entirely from converting only distinct
values, which unique()/match() already does within a call, without the
mutable global state.

Replace the memoized implementation with a plain unique + toTitleCase +
match. Output is identical, and a benchmark on 200k repeated values
shows the simple version is marginally faster than the warm-cache
memoized version (~1.95s vs ~2.26s over 20 runs).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@yihui
yihui marked this pull request as ready for review September 23, 2026 19:06
@yihui
yihui requested a review from LittleBeannie September 23, 2026 19:07
@LittleBeannie
LittleBeannie merged commit 06ee4d0 into main Sep 24, 2026
10 checks passed
@LittleBeannie
LittleBeannie deleted the simplify-titlecase branch September 24, 2026 15:53
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.

2 participants