Patch som tar bort den rekursiva typningen av Entity

[Niklas Gustavsson]

Hejsan

Jag förslår en patch som innebär att vi tar bort (den något skumma)
"rekursiva" typningen av Entity, dvs T i:
public interface Entity<T, ID> { }

Nackdelen med patchen är att sameAs implementationen i AbstractEntity
får en svagare typning. Fördelen är att det nu går att bygga APIer
baserade på interface (som sen implementeras i konkreta klasser för
entiteter och repositories). Det är idag inte möjligt (i alla fall har
inte jag kunna klura ut nåt sätt) på grund av ovanstående
konstruktion.

Den fulla patchen finns på ärendet:
http://code.google.com/p/oppna-program-dao-framework/issues/detail?id=2

Okej att committa?

/niklas

[Anders Asplund]

> Nackdelen med patchen är att sameAs implementationen i AbstractEntity
> får en svagare typning.

Detta kan jag tycka är ett litet problem. Syftet med sameAs är att
kunna likställa två olika entiteter av samma supertyp. I denna lösning
kommer alla entieter med samma id felaktigt likställas.

Skulle följande exempel funka som lösning för ditt problem:

Index: core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/Vehicle.java
===================================================================
--- core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/Vehicle.java (revision 0)
+++ core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/Vehicle.java (revision 0)
@@ -0,0 +1,6 @@
+package se.vgregion.dao.domain.patterns.entity;
+
+import java.util.UUID;
+
+public interface Vehicle extends Entity<Vehicle, UUID> {
+}
Index: core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/VehicleRepository.java
===================================================================
--- core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/VehicleRepository.java (revision 0)
+++ core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/VehicleRepository.java (revision 0)
@@ -0,0 +1,9 @@
+package se.vgregion.dao.domain.patterns.entity;
+
+import java.util.UUID;
+
+import
se.vgregion.dao.domain.patterns.repository.inmemory.AbstractInMemoryRepository;
+
+public class VehicleRepository extends
AbstractInMemoryRepository<Vehicle, UUID> {
+
+}
Index: core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/AbstractVehicle.java
===================================================================
--- core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/AbstractVehicle.java (revision 0)
+++ core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/AbstractVehicle.java (revision 0)
@@ -0,0 +1,15 @@
+package se.vgregion.dao.domain.patterns.entity;
+
+import java.util.UUID;
+
+public abstract class AbstractVehicle extends AbstractEntity<Vehicle,
UUID> implements Vehicle {
+ private UUID id;
+
+ public AbstractVehicle() {
+ id = UUID.randomUUID();
+ }
+
+ public UUID getId() {
+ return id;
+ }
+}
Index: core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/Car.java
===================================================================
--- core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/Car.java (revision 0)
+++ core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/Car.java (revision 0)
@@ -0,0 +1,5 @@
+package se.vgregion.dao.domain.patterns.entity;
+
+public class Car extends AbstractVehicle {
+
+}
Index: core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/MyService.java
===================================================================
--- core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/MyService.java (revision 0)
+++ core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/MyService.java (revision 0)
@@ -0,0 +1,14 @@
+package se.vgregion.dao.domain.patterns.entity;
+
+public class MyService {
+ public static void main(String[] args) {
+ VehicleRepository vehicle = new VehicleRepository();
+ vehicle.store(new Car());
+ vehicle.store(new Bike());
+
+ for (Vehicle v : vehicle.findAll()) {
+ System.out.println(v);
+ }
+
+ }
+}
Index: core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/Bike.java
===================================================================
--- core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/Bike.java (revision 0)
+++ core-bc/composites/types/src/main/java/se/vgregion/dao/domain/
patterns/entity/Bike.java (revision 0)
@@ -0,0 +1,5 @@
+package se.vgregion.dao.domain.patterns.entity;
+
+public class Bike extends AbstractVehicle {
+
+}

[Niklas Gustavsson]

Attachad är en patch som visar på problemet, tar mycket gärna emot
tips kring hur man kan lösa problemet.

Håller i övrigt med om problemet med sameAs, ser dock inte att det
skulle vara svårt att lösa i implementationen (liknande hur man
vanligtvis gör i equals implementationer).

/niklas

2010/11/25 Anders Asplund <aasp...@gmail.com>:

[Anders Asplund]

> Attachad är en patch som visar på problemet, tar mycket gärna emot
> tips kring hur man kan lösa problemet.

Vad tror du om bifogat förslag?

[Niklas Gustavsson]

2010/11/25 Anders Asplund <aasp...@gmail.com>:

>> Attachad är en patch som visar på problemet, tar mycket gärna emot
>> tips kring hur man kan lösa problemet.
> Vad tror du om bifogat förslag?

Har labbat med liknande, men inte så imponerad av att hela APIet blir
fullt med denna typ av konstruktioner:
Vehicle<T extends Vehicle<T>>

Känns som att Entity APIet ger ett allt för stort avtryck.

/niklas

[Anders Asplund]

> Vehicle<T extends Vehicle<T>>

Man kan faktiskt komma undan med:
Vehicle<T> extends Entity<T, UUID>
och
Car extends AbstractEntity<Car> implements Vehicle<Car>

Detta tycker jag här acceptabelt.

Om man vill kan man även dölja Entity genom följande osnygga
konstruktion:
AbstractVehicle<T extends Entity<T, UUID>> extends AbstractEntity<T,
UUID> implements Vehicle<T>

Vilket skulle ge:
Car extends AbstractVehicle<Car>

[Niklas Gustavsson]

On Thu, Nov 25, 2010 at 9:58 AM, Anders Asplund <aasp...@gmail.com> wrote:
>> Vehicle<T extends Vehicle<T>>
>
> Man kan faktiskt komma undan med:
> Vehicle<T> extends Entity<T, UUID>
> och
> Car extends AbstractEntity<Car> implements Vehicle<Car>
>
> Detta tycker jag här acceptabelt.

Var nog lite otydlig, mitt problem är inte antalet tecken (även om det
inte precis är snyggt heller) utan i att T får avtryck överallt där
Vehicle används, eller missuppfattar jag nånting? Tex:
public interface SomeStuff {
void foo(Vehicle<?> vehicle);
}

> Om man vill kan man även dölja Entity genom följande osnygga
> konstruktion:
> AbstractVehicle<T extends Entity<T, UUID>> extends AbstractEntity<T,
> UUID> implements Vehicle<T>
>
> Vilket skulle ge:
> Car extends AbstractVehicle<Car>

Inte säker på att jag alls förstår detta, dock är jag inte intresserad
av klasserna, jag vill ha ett vettigt API på interface nivå.

Om man vänder på frågan: givet att man såklart kan göra en typkoll i
sameAs, vad är fördelen med nuvarande konstruktion?

/niklas

[Anders Asplund]

Om man vänder på frågan: givet att man såklart kan göra en typkoll i
sameAs, vad är fördelen med nuvarande konstruktion?

Kan vi lösa problemet med sameAs så har jag inga problem med patchen. Skulle följande kunna vara ett spår? Har inte haft tid att fullfölja för att se vad det ger men slänger upp det ändå: 

public interface Entity<ID> {

<T> boolean sameAs(T other);

}

I övrigt så har mina förlag varit svar på problemet att det inte går att "bygga APIer baserade på interface" med dagens konstruktion av dao-fwk. Hur vida de blir snygga eller ej är en annan fråga.

//Anders 

[Niklas Gustavsson]

2010/11/25 Anders Asplund <aasp...@gmail.com>:

> Om man vänder på frågan: givet att man såklart kan göra en typkoll i
> sameAs, vad är fördelen med nuvarande konstruktion?
>
> Kan vi lösa problemet med sameAs så har jag inga problem med patchen. Skulle
> följande kunna vara ett spår? Har inte haft tid att fullfölja för att se vad
> det ger men slänger upp det ändå:
> public interface Entity<ID> {
> <T> boolean sameAs(T other);
> }

Inte säker på att det ger oss så mycket, är det inte bara något åt
detta håll vi är ute efter?
public final boolean sameAs(final Entity<ID> other) {
return other != null && other.getClass().equals(getClass()) &&
this.getId().equals(other.getId());
}

> I övrigt så har mina förlag varit svar på problemet att det inte går att
> "bygga APIer baserade på interface" med dagens konstruktion av dao-fwk. Hur
> vida de blir snygga eller ej är en annan fråga.

Jag borde nog varit tydligare med min frågeställning: jag vill kunna
bygga APIer på interface nivå, utan att få en massa avtryck överallt
för att jag valt att använda dao fwk.

/niklas

[Anders Asplund]

> Inte säker på att det ger oss så mycket, är det inte bara något åt
> detta håll vi är ute efter?
> public final boolean sameAs(final Entity<ID> other) {
> return other != null && other.getClass().equals(getClass()) &&
> this.getId().equals(other.getId());
> }

Då var det bara en fråga kvar då :)

Om man tänker sig följande arvshierarkier:

* Entity->Person->Student
* Entity->Vehicle

1. Nuläge
student.sameAs(person) == true:

2. Med patchen utan förändring i sameAs:
vehicle.sameAs(student) == true

3. Med path + ovan nämnda sameAs:
ger endast true om samma type.

Ettan är optimalt om vi kunde få till. Går inte detta så skulle jag nog föredra tvåan då sannolikheten att två entiteter från olika arvshierarkier har samma id får anses minimal. (Om man nu överhuvudtaget skulle få för sig att jämföra fordon och student ;) )

//Anders

[Niklas Gustavsson]

2010/11/25 Anders Asplund <aasp...@gmail.com>:

Ska man i 1 även kunna göra komplementet?
person.sameAs(student) == true

Om ja:

public final boolean sameAs(final Entity<ID> other) {
return other != null

&& (other.getClass().isAssignableFrom(this.getClass()) ||
this.getClass().isAssignableFrom(other.getClass()))
&& this.getId().equals(other.getId());
}

Om nej:

public final boolean sameAs(final Entity<ID> other) {
return other != null &&

other.getClass().isAssignableFrom(this.getClass()) &&
this.getId().equals(other.getId());
}

/niklas

[Niklas Gustavsson]

Jag har committat denna förändring i rev 71, tillsammans med
nedanstående ändring i sameAs (tillsammans med ett par nya testcase
för sameAs).

/niklas

2010/11/26 Niklas Gustavsson <nik...@protocol7.com>:

[Anders Asplund]

Kanon Niklas!

Ska försöka fixa lite i morgon med men livet har kommit emellan ett par dagar nu. :)