Hi Vladimir, After ensuring that the necessary parts were there when replacing the deprecated stuff, I had more time to check if we can remove some superfluous stuff:
1. Regarding MergeableEither, we can remove the explicit "new MergeableEither" instantiation since any Either is converted implicitly to a MergeableEither when you use the merge method. 2. The FetchFeed was working great, but it had many superfluous methods like hashCode and equals. Why maintain them when they are generated automatically in the case classes and all we need is the extractor? Therefore I've only left the unapply method in "object FetchFeed". I also made class FetchFeed abstract- we don't have any FetchFeed instantiations and it doesn't make much sense if it's not a more specific instance (although it would have worked even if it wasn't abstract). Have a nice weekend! Vassil P.S. Fetching feeds doesn't seem to work for me right now, but the reason must be somewhere else, because I've checked that pattern matching works. Does anyone else experience the same problem? On Mon, May 9, 2011 at 2:07 AM, Vladimir Ivanov <[email protected]> wrote: > Thanks for review, Vassil! > > Vladimir > > 2011/5/8 Vassil Dichev <[email protected]> > >> Vladimir, >> >> Nice work- looks good! I've also learned a couple of things from the >> deprecation analysis. >> >> Cheers, >> Vassil >> >> >> On Sun, May 8, 2011 at 3:54 AM, Vladimir Ivanov <[email protected]> >> wrote: >> > Guys, >> > >> > I've just comleted ESME-321: Get rid of deprecated methods. Among other >> > deprecation warnings, there were two I'd like to mention about: >> > >> > 1) Case class FetchFeed had two subclasses: FetchAtom and FetchRss in >> > Action.scala. Since 2.8 version Scala compiler consider it as unduly >> > complicating both usage and implementation and suggests use extractors >> for >> > pattern matching on non-leaf nodes instead. See >> > >> http://scala-programming-language.1934581.n4.nabble.com/Adding-an-extractor-to-an-object-to-behave-like-a-case-object-td1950373.html >> > for >> > more details. Therefore FetchFeed class was rewritten as follows: >> > >> > Before modification: >> > >> > case class FetchFeed(url: UrlStore, tags: List[String]) extends >> Performances >> > >> > After modification: >> > >> > object FetchFeed extends ((UrlStore, List[String]) => FetchFeed) { >> > def unapply(f: FetchFeed): Option[(UrlStore, List[String])] = { >> > Some(f.url, f.tags) >> > } >> > def apply(url: UrlStore, tags: List[String]) = new FetchFeed(url, tags) >> > } >> > >> > class FetchFeed(val url: UrlStore, val tags: List[String]) extends >> > Performances { >> > override def hashCode = 41 * (41 + url.hashCode) + tags.hashCode >> > override def equals(other: Any) = other match { >> > case that: FetchFeed => (that canEqual this) && super.equals(that) && >> > this.url == that.url && this.tags == that.tags >> > case _ => false >> > } >> > def canEqual(other: Any) = other.isInstanceOf[FetchFeed] >> > def copy(url1: UrlStore = url, tags1: List[String] = tags) = new >> > FetchFeed(url1, tags1) >> > } >> > >> > FetchAtom and FetchFeed case classes were left unchanged: >> > >> > case class FetchAtom(override val url: UrlStore, override val tags: >> > List[String]) extends FetchFeed(url, tags) >> > case class FetchRss(override val url: UrlStore, override val tags: >> > List[String]) extends FetchFeed(url, tags) >> > >> > 2) Method Either.merge used to extract value from Either in >> TwitterAPI.scala >> > has been deprecated in Scala 2.8. I replaced it with >> Either.MergeableEither >> > class: >> > >> > Before modification: >> > >> > Either.merge(...) >> > >> > After modification: >> > >> > new Either.MergeableEither(...).merge >> > >> > >> > As always it would be great if someone review these changes. >> > >> > Vladimir >> > >> > >> > >> > 2011/5/2 Richard Hirsch <[email protected]> >> > >> >> I was looking at the looking at 1.3 status this morning and I have a >> >> few questions: >> >> >> >> @Ethan - can we close ESME-328 "Rewrite the comet timeline"? >> >> >> >> @Vladimir is "ESME-332 "There are still some problems with SBT tests" >> >> still valid after the SBT upgrade? >> >> >> >> @Vladimir can we close "ESME-321 Get rid of deprecated methods"? I >> >> wasn't sure how many deprecated methods will still present. >> >> >> >> D. >> >> >> > >> > >> > >> > -- >> > Best Regards, >> > Vladimir Ivanov >> > >> > > > > -- > Best Regards, > Vladimir Ivanov >
