måndagen den 6 oktober 2008 10.15.54 skrev Shawn O. Pearce:
> Robin Rosenberg <robin.rosenberg@xxxxxxxxxx> wrote:
> > diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> > index dfce1b8..3fc5236 100644
> > --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> > +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> > @@ -939,6 +940,33 @@ public String getBranch() throws IOException {
> > }
> >
> > /**
> > + * @return a map with all objects referenced by a peeled ref.
> > + */
> > + public Map<AnyObjectId, List<Ref>> getAllRefsByPeeledObjectId() {
>
> Do we really want to promise List here? Can we make it just
> Collection instead?
Sure. Our promise is actually slightly better, it is Set, but Java doesn't have a suitable class for that.
>
> > + Map<String, Ref> allRefs = getAllRefs();
> > + HashMap<AnyObjectId, List<Ref>> ret = new HashMap<AnyObjectId, List<Ref>>(allRefs.size());
> > + for (Map.Entry<String,Ref> e : allRefs.entrySet()) {
> > + Ref ref = e.getValue();
>
> I think this is cleaner:
>
> for (Ref ref : allRefs.values()) {
>
> as you never use the key.
Yes. I was thinking it might be less efficient, but the JDK implementation looks quite well optimized in 1.6 at least
so values() is slightly faster.
> > + AnyObjectId target = ref.getPeeledObjectId();
> > + if (target == null)
> > + target = ref.getObjectId();
> > + List<Ref> list = ret.get(target);
> > + if (list == null) {
> > + list = Collections.singletonList(ref);
> > + } else {
> > + if (list.size() == 1) {
> > + ArrayList<Ref> list2 = new ArrayList<Ref>(2);
> > + list2.add(list.get(0));
> > + list = list2;
> > + }
> > + list.add(ref);
> > + }
> > + ret.put(target, list);
>
> Hmm. Putting the list every time is pointless. This is may run
> faster because we (on average) only do one hash lookup per target,
> not 2:
>
> List<Ref> nl = Collections.singletonList(ref);
> List<Ref> ol = ret.put(target, nl);
> if (ol != null) {
> if (ol.size() == 1) {
> nl = new ArrayList<Ref>(2);
> nl.add(ol.get(0));
> nl.add(ref);
> ret.put(target, nl);
> } else {
> ol.add(ref)
> ret.put(target, ol);
> }
> }
ok, I guess one just has has to include the comment on why for the casual reader.
-- robin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
