Re: [EGIT PATCH 3/6] Add a method to get refs by object Id
- From
Shawn O. Pearce <spearce@spearce.org>
- Date
- Oct 6, 2008, 22:43 UTC
- Message-ID
- <20081006224348.GB13687@spearce.org>
- In-Reply-To
- <200810070037.53841.robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
Show 12 quoted lines
> måndagen den 6 oktober 2008 10.15.54 skrev Shawn O. Pearce:
> > Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > >
> > > /**
> > > + * @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.java.util.Set? java.util.HashSet?
What am I missing?
Show 11 quoted lines
> > > + 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.I wasn't concerned about performance, I was going for readability. This loop is probably not performance critical as its done once early when the PlotWalk starts. I just found that grabbing the entrySet when all you care about is the values was odd.
Show 15 quoted lines
> > 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. I'm probably too used to this pattern of "put, then test" because I use it a lot when the odds of put returning null are very high. Its an odd idiom. I think most developers wouldn't write it. So I guess a comment of some sort is probably a good idea if you chose to use this mess. ;-)
-- Shawn.