Add fouleggs sample application #105
Conversation
| @@ -43,7 +43,8 @@ class AdminSession extends Actor { | |||
| } | |||
|
|
|||
| def addCastToFilm(personId: Int, filmId: Int, roleName: String): Unit = { | |||
| context.system.actorOf(Props(new CastAndFilmographyFunctor(personId, filmId, roleName, self))) | |||
| val functor: ActorRef = context.system.actorOf(Props(new CastAndFilmographyFunctor(personId, filmId, roleName, self))) | |||
There was a problem hiding this comment.
Props(new SomeActor(params*))This is really not recommended to use in another actor. Please see Akka Documentation - Props for the recommended way to create Props instances, which I really would argue for: Define them in the companion object:
object CastAndFilmographyFunctor {
def props(pId, fId, role, caller) = Props(new CastAndFilmographyFunctor(...))
}This file had been added to the repo on accident. It was meant to be used as a local scratch file.
This avoids the explicit creation of a thread.
|
|
||
| object AdminSession { | ||
|
|
||
| final case class Up() |
There was a problem hiding this comment.
This could be a case object, then you wouldn't need the ().
| object AddCastToFilm { | ||
| final case class Request(personId: Int, filmId: Int, roleName: String) | ||
| final case class Success(personId: Int, filmId: Int, roleName: String) | ||
| final case class Failure(e: Throwable) |
There was a problem hiding this comment.
Those messages do not extend our RequestResponseProtocol. This is nothing bad in general, but they look like they should be subtypes of it (AddCastToFilm defines our Message-Trinity: Request, Success, Failure)!
| override def receive: Receive = commonBehaviour | ||
|
|
||
| def commonBehaviour: Receive = { | ||
| case AdminSession.Up => sender() ! akka.actor.Status.Success |
There was a problem hiding this comment.
I don't get the point of this one. If it's for a health-check, then you could use Identify and ActorIdentity or just rely on the parent-child-relationship and listen in the SystemInitializer for a Terminated message.
Nothing, you have to change now.
|
|
||
| def addCastToFilm(personId: Int, filmId: Int, roleName: String): Unit = { | ||
| log.info(s"Adding person $personId as $roleName to film $filmId") | ||
| val functor: ActorRef = context.system.actorOf(CastAndFilmographyFunctor.props(personId, filmId, roleName, self)) |
| def waitingForFilmInfo: Receive = { | ||
| case DefaultMessagingProtocol.SelectAllFromRelation.Failure(e) => fail(e) | ||
| case DefaultMessagingProtocol.SelectAllFromRelation.Success(relation: Relation) => { | ||
| context.become(waitingForInsertAck orElse commonBehaviour) |
There was a problem hiding this comment.
can you write that at the end of the code block? That's easier to read
| case DefaultMessagingProtocol.SelectAllFromRelation.Success(relation: Relation) => { | ||
| context.become(waitingForInsertAck orElse commonBehaviour) | ||
|
|
||
| val filmInfo: Record = relation.records.get.head |
There was a problem hiding this comment.
Pls use exception-free code (as suggested by Codacy), example:
val filmInfoOption: Option[Record] = relation.records match {
case Some(records) => records.headOption
case None => None
}
filmInfoOption match {
case None => // failure-case use `fail()` or just silently skip next step
case Some(filmInfo) => {
// rest of the code
}
}| val newFilmRecord: Record = Person.Filmography.newRecord( | ||
| Person.Filmography.filmId ~> filmId & | ||
| Person.Filmography.roleName ~> roleName & | ||
| Person.Filmography.filmName ~> filmInfo.get(Film.Info.title).get & |
There was a problem hiding this comment.
avoid record.get(col).get by using the apply-method: record(col): same effect, but nicer
| def waitingForPersonInfo: Receive = { | ||
| case DefaultMessagingProtocol.SelectAllFromRelation.Failure(e) => fail(e) | ||
| case DefaultMessagingProtocol.SelectAllFromRelation.Success(relation: Relation) => { | ||
| val personInfo: Record = relation.records.get.head |
|
|
||
| val newCastRecord: Record = Film.Cast.newRecord( | ||
| Film.Cast.firstName ~> personInfo.get(Person.Info.firstName).get & | ||
| Film.Cast.lastName ~> personInfo.get(Person.Info.lastName).get & |
There was a problem hiding this comment.
same as above: avoid record.get(col).get
|
I created an issue for replacing the |
|
@CodeLionX thanks for the review! Can you have a look at the changes now? |
| case Some(records: Seq[Record]) => records.headOption | ||
| case _ => None | ||
| } | ||
| filmInfoOption match { |
Proposed Changes
fouleggs(think rottentomatoes.com) sample applicationsampleappapplication which is modeled after the case study presented in Shah et al.; ADS: A Manifesto for examples in term paperfouleggsalso serves as an example to explore approaches to generalizeMultiDactorFunctions