0

I hope in a good manner :-) I wrote this piece of code. What I wished to do, is to build something like "cache". I assumed that I had to watch for different threads, as might many calls get to that class, so I tried the ThreadLocal functionality. Base pattern is have "MANY SETS of VECTOR" The vector holds something like: VECTOR.FieldName = "X" VECTOR.FieldValue= "Y" So many Vector objects in a set. Different set for different calls from different machines, users, objects.

 private static CacheVector instance = null;
        private static SortedSet<SplittingVector> s = null;
        private static TreeSet<SplittingVector> t = null;
        private static ThreadLocal<SortedSet<SplittingVector>> setOfVectors = new ThreadLocal<SortedSet<SplittingVector>>();

        private static class MyComparator implements Comparator<SplittingVector> {
     public int compare(SplittingVector a, SplittingVector b) {
         return 1;
     }
     // No need to override equals.
        }

        private CacheVector() {
        }

        public static SortedSet<SplittingVector> getInstance(SplittingVector vector) {
     if (instance == null) {
         instance = new CacheVector();
         //TreeSet<SplittingVector>
         t = new TreeSet<SplittingVector>(new MyComparator());
         t.add(vector);
         s = Collections.synchronizedSortedSet(t);//Sort the set of vectors
         CacheVector.assign(s);
     } else {
         //TreeSet<SplittingVector> t = new TreeSet<SplittingVector>();
         t.add(vector);
         s = Collections.synchronizedSortedSet(t);//Sort the set of vectors
         CacheVector.assign(s);
     }
     return CacheVector.setOfVectors.get();
        }

        public SortedSet<SplittingVector> retrieve() throws Exception {
     SortedSet<SplittingVector> set = setOfVectors.get();
     if (set == null) {
         throw new Exception("SET IS EMPTY");
     }
     return set;
        }

        private static void assign(SortedSet<SplittingVector> nSet) {
     CacheVector.setOfVectors.set(nSet);
        }

So... I have it in the attach and I use it like this:

CachedVector cache = CachedVector.getInstance(bufferedline);

The nice part: Bufferedline is a splitted line based on some delimiter from data files. Files can be of any size.

So how do you see this code? Should I be worry ? I apologise for the size of this message!

Nick Bastin
  • 30,415
  • 7
  • 59
  • 78
hephestos
  • 434
  • 1
  • 6
  • 19

1 Answers1

0

Writing correct multi-threaded code is not that easy (i.e. your singleton fails to be), so try to rely on existing solutions if posssible. If you're searching for a thread-safe Cache implementation in Java, check out this LinkedHashMap. You can use it to implement a LRU cache. And collections.synchronizedMap(). can make this thread-safe.

b_erb
  • 20,932
  • 8
  • 55
  • 64
  • Is it possible to describe in natural way, what is the fault on this code? I am assuming at least the singleton, as singleton should be ok. – hephestos Nov 26 '10 at 12:34
  • So something after that goes wacko. In the docs however, Vectors are synchronized themselves...so shouldn't I be safe to just load synchronized classes with a "tread-session" ? – hephestos Nov 26 '10 at 12:36