bengbengbalabalabeng commented on issue #989:
URL: https://github.com/apache/fesod/issues/989#issuecomment-5254803014
I completely agree with the proposed refactoring to split `ClassUtils` by
responsibility and optimize the caching logic. Breaking down the monolithic
`ClassUtils` class delivers huge gains in code readability, maintainability,
and separation of concerns.
Here are a few thoughts and technical suggestions regarding the refactoring
design for further discussion:
#### 1. Keep existing `public` methods on `ClassUtils` as a unified Facade
API that delegates directly to the new internal resolvers underneath, **without
marking them as `@Deprecated`**. Here is why:
- `ClassUtils` serves as a well-established entry point across the
framework. Preserving its existing public method signatures ensures **zero
breaking changes** for downstream users.
- `ClassUtils` naturally functions as a public-facing facade. Encapsulating
complex parsing and caching details inside specialized resolvers while leaving
`ClassUtils` solely responsible for routing is a clean, standard architectural
design. There is no need to force users to migrate to internal classes.
#### 2. The newly extracted classes should be scoped as
**`package-private`** (accessible only to `ClassUtils` and other
package-internal components) to avoid polluting the public API surface.
Proposed naming:
- **`SheetHeadFieldResolver`** (equivalent to `FieldCacheUtils`):
Responsible for column layout, field mappings (`FieldCache`), and index/order
calculations for sheet headers.
- **`SheetContentPropertyResolver`** (equivalent to
`ExcelContentPropertyUtils`): Responsible for cell content formatting
(`ExcelContentProperty`), converters, and styling.
#### 3. To eliminate duplicated branches and procedural boilerplate, we can
replace the repetitive `switch` statements with an interface-based strategy
pattern combined with an `EnumMap`:
##### Cache Strategy:
```java
public interface MetadataCacheStrategy<K, V> {
V get(K key, Function<K, V> mappingFunction);
void clear();
// In-memory cache
class MemoryCache<K, V> implements MetadataCacheStrategy<K, V> {
private final Map<K, V> cache;
public MemoryCache() {
this(new ConcurrentHashMap<>());
}
public MemoryCache(Map<K, V> cache) {
this.cache = cache;
}
@Override
public V get(K key, Function<K, V> mappingFunction) {
return cache.computeIfAbsent(key, mappingFunction);
}
@Override
public void clear() {
cache.clear();
}
}
// ThreadLocal cache
class ThreadLocalCache<K, V> implements MetadataCacheStrategy<K, V> {
// ......
}
// No-op cache
class NoOpCache<K, V> implements MetadataCacheStrategy<K, V> {
// ......
}
}
```
##### Usage Example (taking `SheetHeadFieldResolver` as an example):
```java
// Package-private
final class SheetHeadFieldResolver {
private static final Map<CacheLocationEnum,
MetadataCacheStrategy<FieldCacheKey, FieldCache>> CACHE_STRATEGIES;
static {
Map<CacheLocationEnum, MetadataCacheStrategy<FieldCacheKey,
FieldCache>> map = new EnumMap<>(CacheLocationEnum.class);
map.put(CacheLocationEnum.MEMORY, new
MetadataCacheStrategy.MemoryCache<>());
map.put(CacheLocationEnum.THREAD_LOCAL, new
MetadataCacheStrategy.ThreadLocalCache<>());
map.put(CacheLocationEnum.NONE,
MetadataCacheStrategy.NoOpCache.instance());
CACHE_STRATEGIES = Collections.unmodifiableMap(map);
}
public static FieldCache resolve(Class<?> clazz, ConfigurationHolder
configurationHolder) {
CacheLocationEnum location =
configurationHolder.globalConfiguration().getFiledCacheLocation();
MetadataCacheStrategy<FieldCacheKey, FieldCache> cache =
CACHE_STRATEGIES.get(location);
return cache.get(new FieldCacheKey(clazz, configurationHolder), key
-> doResolve(clazz, configurationHolder));
}
private static FieldCache doResolve(Class<?> clazz, ConfigurationHolder
configurationHolder) {
// ......
}
// Clear ThreadLocal cache only by default
public static void clearCache(boolean clearAll) {
CACHE_STRATEGIES.get(CacheLocationEnum.THREAD_LOCAL).clear();
if (clearAll) {
CACHE_STRATEGIES.get(CacheLocationEnum.MEMORY).clear();
}
}
}
```
#### After refactoring:
```java
public class ClassUtils {
public static FieldCache declaredFields(Class<?> clazz,
ConfigurationHolder configurationHolder) {
return SheetHeadFieldResolver.resolve(clazz, configurationHolder);
}
public static ExcelContentProperty declaredExcelContentProperty(
Map<?, ?> dataMap, Class<?> headClazz, String fieldName,
ConfigurationHolder configurationHolder) {
return SheetContentPropertyResolver.resolve(dataMap, headClazz,
fieldName, configurationHolder);
}
// ......
public static void removeThreadLocalCache() {
SheetHeadFieldResolver.clearCache(false);
SheetContentPropertyResolver.clearCache(false);
}
public static List<Class<?>> getAllInterfaces(final Class<?> cls) { ... }
}
```
Feedback and thoughts are welcome ：）
--
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]
