Refactoring SFX to JFX conversions

61 views
Skip to first unread message

Rafael Afonso

unread,
Aug 24, 2014, 9:39:46 PM8/24/14
to scala...@googlegroups.com
Greetings:

following the issue 153, I deceided to make some refactorings that I should have done a long time ago. Nowadays, each companion object has its own ScalaFX to JavaFX implicit conversor. However, these methods consists merely in receive a SFXDelegate and return its respective delegate. So, I created the trait SFXDelegateCompanion, which contains the implicit method sfxObject2jfx:

trait SFXDelegateCompanion[D <: java.lang.Object] {

 
/**
   * Converts a ScalaFX object to its JavaFX equivalent. Given a SFXDelegate, this method returns
   * its delegate. If the argument was `null` it returns also `null`.
   *
   * @param s SFXDelegate object to be converted.
   * @return Wrapped JavaFX object or `null` if argument is `null`.
   */

 
implicit def sfxObject2jfx[S <: SFXDelegate[D]](s: S): D =
   
if (s == null) null.asInstanceOf[D]
   
else s.delegate

}

Realize that the null verification is already being done in converter. 
This way a object companion such as Styleable changes from 

 object Styleable {

 
implicit def sfxStyleable2jfx(s: Styleable): jfxcss.Styleable = if (s != null) s.delegate else null

}

to 

object Styleable
 
extends SFXDelegateCompanion[jfxcss.Styleable]

  
So now, I changed the objects in animation, css, swing, event, transform, web, geometry, paint, stage and converter packages. All tests are running well. However, I discovered one problem with classes which uses generics. For example, for EventType, its companion would be like 

object Styleable
 
extends SFXDelegateCompanion[jfxe.EventType[_]] // No type specified
  
When I runned the tests, the implicit conversor was null, generating error in test. Therefore I am skipping the classes with generics at the moment. In future I (or someone else) will investigate why it is happening and how to deal with it.
If someone had no objection, I will syncronyze with GitHub repository at tuesday.

Thanks,

Rafael Afonso

Jarek Sacha

unread,
Aug 25, 2014, 10:27:33 PM8/25/14
to scala...@googlegroups.com
Rafael,

I like the idea of reducing code duplication in companion objects. Though the issue with the companion objects for classes with parametric types may not be resolvable. An object is a concrete instance of a class and has to have type specified; it's type cannot be parametrized. The solution you are proposing, in practice, would apply only to part of the code. The question for me is if it is better to have at least part of the code simplified or is it better to have coding consistency (or consistent implicit conversion pattern). I will probably lean toward consistency in this case, even if it requires some additional code. That duplicated code for implicit conversions is covered by automated tests that that detect if the code is present and correctly implemented. So we have some guarantee of correctness for the duplicated (or partially duplicated code).

On the other hand if there is a solution that would apply to all (or almost all :) cases I would be for it. There may be some trick that is not apparent to me.

Jarek
--
You received this message because you are subscribed to the Google Groups "ScalaFX Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scalafx-dev...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Rafael Afonso

unread,
Aug 27, 2014, 7:27:08 PM8/27/14
to scala...@googlegroups.com
Jarek:

So is your purpose to wait until to get a generic approach, which contemplates parametric types? We could create a prototype project to mock our situation and ask for help among most experienced people (StackOverflow?).

Thanks,

Rafael Afonso
To unsubscribe from this group and stop receiving emails from it, send an email to scalafx-dev+unsubscribe@googlegroups.com.

Jarek Sacha

unread,
Aug 27, 2014, 8:30:42 PM8/27/14
to scala...@googlegroups.com
Rafael,

On 8/27/2014 7:27 PM, Rafael Afonso wrote:
> Jarek:
>
> So is your purpose to wait until to get a generic approach, which
> contemplates parametric types?
I was just responding to your request for an opinion. I think that
proposed solution would apply to only some companion objects for classes
that do not use parameter types. I think that having it partially solved
creates a potential difficulty in understating the code base. In my
opinion, it is better to have more uniform approach across the whole
code base than spacial cases. Especially that benefit of changes is
relatively small - removal of one implicit conversion per wrapper that
does not use parametric types. Just my opinion.

> We could create a prototype project to mock our situation and ask for
> help among most experienced people (StackOverflow?).
A good idea. There may be a solution that would cover all the cases,
thus actually simplifying the code uniformly across all wrappers.

Jarek

>
> Thanks,
>

Rafael Afonso

unread,
Aug 30, 2014, 8:14:29 PM8/30/14
to scala...@googlegroups.com
I reverted all my changes. Actually, I simply renamed my scalafx folder and re-cloned the project from Github. This way, when it was possible, I could reuse my original code.

Thanks,

Rafael Afonso

Massimo Redaelli

unread,
Nov 27, 2014, 11:57:34 AM11/27/14
to scala...@googlegroups.com

Yesterday I had a conversation with a guy from Typesafe, and with his help I would make the following suggestion:

1) Add a type member in SFXDelegate:

trait SFXDelegate[+D <: Object] extends AnyRef {

  type Delegate = D // !!

  /**
   * JavaFX object to be wrapped.
   */
  def delegate: Delegate

   // ...
}

Companion

trait SFXCompanion {
  implicit def sfx2jfx[D <: SFXDelegate[_]](s: D): s.Delegate =
    if (s == null) null.asInstanceOf[s.Delegate]
    else s.delegate
}

And then the companions:

object TreeTableView extends SFXCompanion {

 // ...

I didn't run the tests, but playing around with simplified cases in REPL seems to be working.

Sorry if this is stupid: I'm way to young, Scala-wise :P

M.

Jarek Sacha

unread,
Nov 27, 2014, 12:31:55 PM11/27/14
to scala...@googlegroups.com
Massimo,

Looks interesting. I can't say that I fully understand what is going on, but it looks it captures the parameter type in internal type, somehow delaying decision of what that type really is. This possibly improves what Rafael suggested and may get the companion objects right.

Just saw recently interesting presentation "Introduction to Dependent Types in Scala" that seems to use similar technique to modify types returned by functions depending on argument passed. Still need to wrap my head around how this works and how it could be utilized in ScalaFX. I was thinking about fixing issue with nested implicit conversions (where you need to convert a type and type parameters of that type). This may be a better place to start.

Your suggestion looks like good starting point. Would you like to test it on the actual code base of ScalaFX? It may be a couple of days before I will be able to get to it.

Jarek

Massimo Redaelli

unread,
Nov 28, 2014, 4:50:07 AM11/28/14
to scala...@googlegroups.com
I will. Can't promise it will be fast though :)

M.

--
You received this message because you are subscribed to the Google Groups "ScalaFX Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scalafx-dev...@googlegroups.com.

Massimo Redaelli

unread,
Nov 28, 2014, 11:36:43 AM11/28/14
to scala...@googlegroups.com

Hmmm.

The

trait SFXDelegate[+D <: Object] extends AnyRef {

  type Delegate = D

statement compiles fine in 2.10 but fails in 2.11. Something with covariance and invariance :’(

So it would become something like

trait SFXDelegate[+D <: Object] extends AnyRef
 {

  type Delegate <: D
  // ...

but then also we must define Delegate

class TreeTableView [T](override val delegate: jfxsc.TreeTableView [T]) extends SFXDelegate[jfxsc.TreeTableView [T]] { 

  type Delegate = jfxsc.TreeTableView [T] // new
  // ...
}

which is becoming quite tiresome to write (and I still get some errors).

I’ll try some more: I have little idea what I am doing, but it’s a great way to learn some Scala :P

M

Reply all
Reply to author
Forward
0 new messages