These are chat archives for ceylon/ceylon-ide-eclipse

15th
Mar 2016
Gavin King
@gavinking
Mar 15 2016 13:58
OK, so I have some serious feedback here
this kind of thing:
    shared interface EditorData {
        shared formal List<CommonToken>? tokens;
        shared formal Tree.CompilationUnit? rootNode;
        shared formal Node? node;
        shared formal VirtualFile? sourceVirtualFile;
    }
is an abuse of optional types
for the refactoring to be useful, all these things need to exist
you’re forcing me to check them all every time at the use-site
which is painful
David Festal
@davidfestal
Mar 15 2016 13:59
are you sure about that ?
Gavin King
@gavinking
Mar 15 2016 13:59
instead of checking them where they’re initialized
and then go and check the initialization
David Festal
@davidfestal
Mar 15 2016 13:59
I've dive very deeply in the Java code before doing this
Gavin King
@gavinking
Mar 15 2016 13:59
the initialization actually assumes they exist!
David Festal
@davidfestal
Mar 15 2016 13:59
in in some case they were, in some cases they were not
Gavin King
@gavinking
Mar 15 2016 13:59
implicitly
tokens and rootNode for sure must be non-null
David Festal
@davidfestal
Mar 15 2016 13:59
many many implicits in this Java code yes
Bastien Jansen
@bjansen
Mar 15 2016 13:59
those are the result of the parsing phase, so I would expect them to exist in every case
Gavin King
@gavinking
Mar 15 2016 13:59
not 100% sure about the other two
so it means you have bullshit assertions all over the place
instead of proper existence checking / failure handling
that’s terrible
we need much more discipline in that
David Festal
@davidfestal
Mar 15 2016 14:00
remember it's Java code translated to Ceylon
Gavin King
@gavinking
Mar 15 2016 14:00
b/c it makes the Ceylon code a mess
David Festal
@davidfestal
Mar 15 2016 14:00
and also Java design
Gavin King
@gavinking
Mar 15 2016 14:01
Ceylon code with lots of assertions and existence checks is painful
David Festal
@davidfestal
Mar 15 2016 14:01
initially with much less restrictions
Gavin King
@gavinking
Mar 15 2016 14:01
i understand that
but somebody wrote that interface
by hand
it wasn’t generated
Bastien Jansen
@bjansen
Mar 15 2016 14:01
it's possible the Java code has lots of if ( != null) "just to be sure", but in practice things are always non-null
David Festal
@davidfestal
Mar 15 2016 14:01
and I wanted, at least at start, have a structure that would be comparable to to Java code
I wrote it
but in the exact purpose of keeping the Java initial structure as much as possible to enable tracking between Java and Ceylon code
in case of abstraction bugs
Gavin King
@gavinking
Mar 15 2016 14:02
@bjansen right, but that’s b/c in Java the typechecker never helped with this stuff
Bastien Jansen
@bjansen
Mar 15 2016 14:02
sure
Gavin King
@gavinking
Mar 15 2016 14:02
well it made my life extra-special-painful
David Festal
@davidfestal
Mar 15 2016 14:02
knowing that afterwards we can refactor things much better once in Ceylon
Gavin King
@gavinking
Mar 15 2016 14:02
when i tried to port extract function
Bastien Jansen
@bjansen
Mar 15 2016 14:02
I wasn't saying I approve this, I was trying to find a plausible explanation
Gavin King
@gavinking
Mar 15 2016 14:03
@davidfestal I have always disapproved of that strategy
David Festal
@davidfestal
Mar 15 2016 14:03
Happy to hear this
:-)
Gavin King
@gavinking
Mar 15 2016 14:03
or rather: it’s OK, to an extent
but once you start introducing new abstractions
that are incorrect
that’s time to maybe rethink
David Festal
@davidfestal
Mar 15 2016 14:04
I agree on the principle
in practice I probably didn't see the "border" at the right place
Gavin King
@gavinking
Mar 15 2016 14:05
I also strongly suspect that we can’t refactor w/o a IFileVirtualFile, a Document, and a Project
Bastien Jansen
@bjansen
Mar 15 2016 14:05
we already have virtual files
and an interface that allows manipulating documents
DocumentChanges
with text edits
David Festal
@davidfestal
Mar 15 2016 14:06
@gavinking : at the time this was done, the Virtual file abstraction was not what it is now
look, this is an incremental work by nature
sometimes you refactor on the content of a document
sometimes on a file on disk
I had found both cases at the time I worked on that
afair
Bastien Jansen
@bjansen
Mar 15 2016 14:10
I agree with David, we've already abstracted a lot of things, but it's hard to think of every possible way to do things, so we add "meta-abstractions" from time to time
which means a feature that we abstracted some time ago could have used that meta-abstraction, but we didn't have it at that time
David Festal
@davidfestal
Mar 15 2016 14:11
@gavinking : In fact these wrong abstractions such as EditorData reflect the ambiguities or implicits that were in the Java code
it seems not a bad thing to me to have them at first
at least we see what was wrong (or at least too implicit for Ceylon) in the Java code
but then, in a second step, once it has put in light the various use cases the Java code allows,
then we can try to find a solution a refactor deeply and restructure
Bastien Jansen
@bjansen
Mar 15 2016 14:13
also keep in mind that this was the first and only refactoring that we abstracted, something like a draft, so obviously as soon as we'd add more refactorings, there would be changes to do to make cleaner abstractions
David Festal
@davidfestal
Mar 15 2016 14:13
at least that my motivation when doing that
Gavin King
@gavinking
Mar 15 2016 14:13
but it undermines my ability to be able to understand the whole lifecycle of these objects
Bastien Jansen
@bjansen
Mar 15 2016 14:13
I think we lost him :)
Gavin King
@gavinking
Mar 15 2016 14:13
and what is known, and what is not known
David Festal
@davidfestal
Mar 15 2016 14:14
the current ceylon reflects the fact that, in the Java code, some of the data were not necessarily existing in all use cases
Gavin King
@gavinking
Mar 15 2016 14:15
I am aware of that
David Festal
@davidfestal
Mar 15 2016 14:16
but sure you're certainly much more aware of me of these various use cases since you wrote this code
Gavin King
@gavinking
Mar 15 2016 14:21
that’s not the point
the point was that instead of having these things fail early
“oh, I don’t have enough information to refactor"
they failed in thousands of little places
David Festal
@davidfestal
Mar 15 2016 14:23
understand
now we have to possibility to change this
sure
and sorry if I did wrong
Gavin King
@gavinking
Mar 15 2016 14:33
ceylon/ceylon-ide-eclipse#1731
Bastien Jansen
@bjansen
Mar 15 2016 14:33
I noticed that one too, forgot to open an issue