bengbengbalabalabeng commented on issue #989:
URL: https://github.com/apache/fesod/issues/989#issuecomment-5282510729
Hi, @nkuprins
The following only discusses the differences from the existing
implementation and the reasons behind such design.
> 3.1 ... it's no longer the same object those fields point at.
In my earlier design, I believed that `ClassUtils` no longer needed to hold
any public/private cache fields, and that caching could be fully delegated to
private implementations within `SheetHeadFieldResolver` and
`SheetContentPropertyResolver`. However, this approach did not fully account
for the compatibility issues of downstream users who may rely on these public
fields.
To meet compatibility requirements, a compromise approach can be adopted:
- The cache field is still not held directly by `ClassUtils`;
- However, `ClassUtils` retains its original public access entry points,
exposing an immutable view through delegation;
- The actual cache is maintained internally by the `*Resolver`.
```java
final class SheetHeadFieldResolver {
// ......
static Map<...> getFieldCache() {
return Collections.unmodifableMap(...);
}
}
class ClassUtils {
public static Map<...> getFieldCache() {
return SheetHeadFieldResolver.getFieldCache();
}
}
```
Additionally, the overloaded constructor of
MetadataCacheStrategy.InMemoryCache is still retained, mainly to provide
flexibility for extending other Map types (e.g., ConcurrentReferenceHashMap) in
the future. (This also applies to the overload of
MetadataCacheStrategy.ThreadLocalCache.)
> 3.2 On clearCache(boolean clearAll) with true argument.
The `clearAll` parameter in `clearCache(boolean clearAll)` is mainly
designed for unit test scenarios, used to reset all caches during testing. Of
course, this method could also be split into two more explicit APIs, with the
calls delegated uniformly by `ClassUtils`:
```java
public static void removeThreadLocalCache() {
SheetHeadFieldResolver.clearThreadLocalCache();
SheetContentPropertyResolver.clearThreadLocalCache();
}
public static void removeInMemoryCache() {
SheetHeadFieldResolver.clearInMemoryCache();
SheetContentPropertyResolver.clearInMemoryCache();
}
```
If there are any other points to consider, 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]
