nkuprins opened a new issue, #989:
URL: https://github.com/apache/fesod/issues/989

   ### Search before asking
   
   - [x] I searched in the [issues](https://github.com/apache/fesod/issues) and 
found nothing similar.
   
   
   ### Motivation
   
   `fesod-sheet/src/main/java/org/apache/fesod/sheet/util/ClassUtils.java` is 
~600 lines and holds three unrelated responsibilities in one class. It was 
really hard to navigate when tracing how a column or a cell format is resolved. 
Concretely:
   
   **1. Column-layout resolution** - which declared fields become spreadsheet 
columns, and in what
   order:
   
   - `declaredFields` (L285), `doDeclaredFields` (L307), `resortField` (L394),
     `buildSortedAllFieldMap` (L449), `declaredOneField` (L472), 
`FieldCacheKey` (L572)
   - caches: `FIELD_CACHE` (L76), `FIELD_THREAD_LOCAL` (L80)
   
   **2. Per-field formatting resolution** - converter, date/number format and 
style, merged from the
   head class and the row's actual runtime class:
   
   - `declaredExcelContentProperty` (L114), `getExcelContentProperty` (L126),
     `doGetExcelContentProperty` (L149), `combineExcelContentProperty` (L168), 
`buildKey` (L193),
     `declaredFieldContentMap` (L197), `doDeclaredFieldContentMap` (L224), 
`ContentPropertyKey` (L565)
   - caches: `CONTENT_CACHE` (L97), `CONTENT_THREAD_LOCAL` (L103), 
`CLASS_CONTENT_CACHE` (L85),
     `CLASS_CONTENT_THREAD_LOCAL` (L91)
   
   **3. `getAllInterfaces`** (L530/L547) -  a generic reflection helper 
unrelated to either of the
   above. It is a trimmed copy of 
`org.apache.commons.lang3.ClassUtils.getAllInterfaces`, and its
   only caller is `FieldUtils.getField` (`FieldUtils.java:165`).
   
   On top of that, concerns (1) and (2) each cache their result per 
`CacheLocationEnum`
   (`THREAD_LOCAL` / `MEMORY` / `NONE`), and the switch that dispatches on the 
cache location is
   repeated three times - L128-146, L202-221 and L286-304. 
   The three differ only in the map and key types. The branch order, the lazy 
`ThreadLocal` init block and the 
   `default: throw new UnsupportedOperationException("unsupported enum")` are 
identical.
   
   ### Solution
   
   Split the file four ways within `org.apache.fesod.sheet.util`, as a pure 
move - no logic changes:
   
   1. **`FieldCacheUtils`** - column-layout resolution: `declaredFields`, 
`doDeclaredFields`, `resortField`, `buildSortedAllFieldMap`, 
`declaredOneField`, `FieldCacheKey`, plus its two cache fields.
   2. **`ExcelContentPropertyUtils`** - per-field formatting resolution: 
`declaredExcelContentProperty`, `getExcelContentProperty`, 
`doGetExcelContentProperty`, `combineExcelContentProperty`, `buildKey`, 
`declaredFieldContentMap`, `doDeclaredFieldContentMap`, `ContentPropertyKey`, 
plus its four cache fields.
   3. **`CacheLocationUtils`** - one generic method replacing the three
      switches:
   
      ```java
      static <K, V> V computeIfAbsent(
              CacheLocationEnum cacheLocation,
              ThreadLocal<Map<K, V>> threadLocalCache,
              Map<K, V> memoryCache,
              Supplier<K> keySupplier,
              Supplier<V> valueSupplier) { ... }
      ```
   
   4. **`ClassUtils`** - trimmed to `getAllInterfaces` plus 
`removeThreadLocalCache`, kept as a public entry point so its two callers don't 
change.
   
   The two clusters are almost disjoint: neither calls into the other and they 
share no private helper. The one exception is `removeThreadLocalCache` (L591), 
which clears all three `ThreadLocal`s and so spans both clusters. It has two 
production callers (`ExcelAnalyserImpl.java:298`, `WriteContextImpl.java:567`), 
so it stays on `ClassUtils` as a delegate to a package-private clear method on 
each of the two new classes - that keeps both call sites untouched. Apart from 
that one method, the partition needs no duplication and no new coupling.
   
   ### Alternatives
   
   _No response_
   
   ### Anything else?
   
   **One caveat.** `ClassUtils` is a public utility class - some things to be 
moved are `public static`. If refactoring takes place, not sure if 
`@Deprecated` would be needed instead of just removing.
   
   ### Are you willing to submit a PR?
   
   - [x] I'm willing to submit a PR!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to