JFXtras / jfxtras

A supporting library for JavaFX, containing helper classes, extended layouts, controls and other interesting widgets.
http://jfxtras.org
Other
599 stars 123 forks source link

changed default Appointment setters and getters to public default #42

Closed daviddbal closed 8 years ago

daviddbal commented 8 years ago

This should change the scope of the default methods so that a implementation outside the package won't have to define those methods.

tbee commented 8 years ago

Ok. Interesting.... "All members of interfaces lacking access modifiers are implicitly public." http://docs.oracle.com/javase/specs/jls/se8/html/jls-6.html#jls-6.6.1

So this should not matter? Or is a default method different? I cannot find any documentation on that default methods are different.

Why do you need this change?

daviddbal commented 8 years ago

I think default methods are package scope. I'll try to find some documentation.

daviddbal commented 8 years ago

Try this as a test. Make a class that implements Appointment and then make stubs for all the necessary methods. You will see that all methods are made - even the default ones.

daviddbal commented 8 years ago

Of course, that test must be done in a project that has JFxtras outside its package.

daviddbal commented 8 years ago

I think the defaults should be public. But, I see a problem sometimes. I don't know if I did something wrong, Eclipse is making the mistake or if something is wrong with Java.

With jfxtras-agenda-8.0.r4.jar A empty class is as follows:

`import jfxtras.scene.control.agenda.Agenda.Appointment; import jfxtras.scene.control.agenda.Agenda.AppointmentGroup;

public class A implements Appointment {

@Override
public AppointmentGroup getAppointmentGroup() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public String getDescription() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public String getLocation() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public String getSummary() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public Boolean isWholeDay() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public void setAppointmentGroup(AppointmentGroup s) {
    // TODO Auto-generated method stub

}

@Override
public void setDescription(String s) {
    // TODO Auto-generated method stub

}

@Override
public void setLocation(String s) {
    // TODO Auto-generated method stub

}

@Override
public void setSummary(String s) {
    // TODO Auto-generated method stub

}

@Override
public void setWholeDay(Boolean b) {
    // TODO Auto-generated method stub

}

}`

with jfxtras-agenda-8.0-r5.SNAPSHOT.jar I get all methods - defaults too.

`import java.time.LocalDateTime; import java.time.ZonedDateTime; import java.util.Calendar;

import jfxtras.scene.control.agenda.Agenda.Appointment; import jfxtras.scene.control.agenda.Agenda.AppointmentGroup;

public class A implements Appointment {

@Override
public AppointmentGroup getAppointmentGroup() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public String getDescription() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public String getLocation() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public String getSummary() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public Boolean isWholeDay() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public void setAppointmentGroup(AppointmentGroup s) {
    // TODO Auto-generated method stub

}

@Override
public void setDescription(String s) {
    // TODO Auto-generated method stub

}

@Override
public void setLocation(String s) {
    // TODO Auto-generated method stub

}

@Override
public void setSummary(String s) {
    // TODO Auto-generated method stub

}

@Override
public void setWholeDay(Boolean b) {
    // TODO Auto-generated method stub

}

@Override
public Calendar getStartTime() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public void setStartTime(Calendar c) {
    // TODO Auto-generated method stub

}

@Override
public Calendar getEndTime() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public void setEndTime(Calendar c) {
    // TODO Auto-generated method stub

}

@Override
public ZonedDateTime getStartZonedDateTime() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public void setStartZonedDateTime(ZonedDateTime v) {
    // TODO Auto-generated method stub

}

@Override
public ZonedDateTime getEndZonedDateTime() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public void setEndZonedDateTime(ZonedDateTime v) {
    // TODO Auto-generated method stub

}

@Override
public LocalDateTime getStartLocalDateTime() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public void setStartLocalDateTime(LocalDateTime v) {
    // TODO Auto-generated method stub

}

@Override
public LocalDateTime getEndLocalDateTime() {
    // TODO Auto-generated method stub
    return null;
}

@Override
public void setEndLocalDateTime(LocalDateTime v) {
    // TODO Auto-generated method stub

}

}`

tbee commented 8 years ago

Possibly the RetroLamba I included for making JFXtras run on Android may be a factor... That will do migrate default methods as well. But I though it was not enabled on Agenda. Interesting. I'll take a peek.

Tom

tbee commented 8 years ago

I created a new project in Eclipse, included the r5-snapshot JAR files as they are generated by Gradle and created the following method in a class in the root package.

public static void main(String[] args) {
    Agenda.Appointment a = new Agenda.AppointmentImplLocal();
    a.getStartTime();
}

Running this results in the "not implemented" exception as is visible in the default method's implementation. So the only conclusion can be that the default methods are public (as per specification) and this PR is not necessary...

daviddbal commented 8 years ago

Yes, the default methods are public so the PR won't make a difference. However, it still remains that in r5-snapshop they are not acting as default methods. Have you verified the problem exists? Is retrolambda the cause? If so do you have an idea on how to fix it? Do you want to fix it?

It is not a big problem. I simply declare the default methods again in my class, but I shouldn't have to do that. I read a rumor that Java 8 will be ported to Android soon. If so then retrolambda would become obsolete and then the problem will go away, right?

tbee commented 8 years ago

I used the actual jar files, which are retrolambaded, and I could not verify the problem. I will look at it once more, to see if I made a slip somewhere. OTOH it does not hurt to explicitly make them public, no harm in that.

I wrote a blog post about Java and Android, and I have not heard that rumor (and I talk to a lot of inside people in Oracle). https://tbeernot.wordpress.com/2015/11/05/i-am-worried/

tbee commented 8 years ago

Well, if it helps, I'll just commit this :-)

daviddbal commented 8 years ago

I don't think committing the change will help. Initially I thought the problem I believe your test shows the default methods are public. The actual problem is that they are not acting as if they are defined as default. I am required to implement the methods anyway.

For some reason I am not getting the new change (add explicit public) so I can't test it. I deleted the Gradle cache and downloaded a new version. I still see the former version without public.

I tried the same experiment with the JFXTras 8.0 Samples jar, which is named jfxtras-labs-samples-8.0-r5-SNAPSHOT-shadow, and I didn't have the problem with the defaults. I don't understand the problem.

The Android rumor came from this site: http://sseblog.ec-spride.de/2015/03/lambda-expressions-android/

tbee commented 8 years ago

So I should revert this commit?

Well that Android news would be extremely good. Very late to the party, but good.

daviddbal commented 8 years ago

I think you should revert the commit. It doesn't do anything.

I would like to get to the bottom of the cause of my problem. The most important question is if the problem is unique to me or not?

I have a test that should determine the scope of the problem. Can you please checkout jfxtras-labs and go to jfxtras-labs/src/test/java/jfxtras/labs/repeatagenda/AppointmentImplTest.java. That class shows all the methods Eclipse requires me to add when I implement Appointment. Please delete those methods and see which methods you are required to add. If the methods you add don't include the defaults then the problem is uniquely mine. If you also have the defaults then the problem is universal.

By the way, I think you are right that Google would be a better home for Java.