BaseList (and all other custom List implementations) refactor

[avlo]

hi, eric. per your consideration, upon careful attention to @guilherme comment in BaseList.java, commit 2803d1111af47069d7470e6917c7d714d527f3d0:

diff --git a/nostr-java-event/src/main/java/nostr/event/list/BaseList.java b/nostr-java-event/src/main/java/nostr/event/list/BaseList.java
index f23837a..054c9d4 100644
--- a/nostr-java-event/src/main/java/nostr/event/list/BaseList.java
+// TODO: Why are we using this instead of just use a regular java collection?      <--- @guilherme's comment

i think he is absolutely correct. it appears the existence of BaseList custom List implementation is exclusively used to enforce uniqueness when adding items to a list:

@Override
public void add(@NonNull T elt) {
    if (!list.contains(elt)) {       <------ uniqueness enforcement
        this.list.add(elt);
    }
}

which instead, can- and should- be achieved by using Set rather than List.

in general, extending List/Collection implementations are commonly considered an anti-pattern (turns out, commonly known as pseudo-type) for the following reasons among others:

• lead to incompatibilities with standard list/collection types while the base type (List<...>, Set<...>) would've worked in all cases.
• customized constructor/method implementations lead to futher/required considerations which must be implemented by the extending class
• necessitate commensurate unit-tests and extra testing overhead solely for the custom implementations

to that end, i experimented with a quick refactoring of all code requiring custom List implementations:

nostr-java-base/src/main/java/nostr/base/FNostrList.java
nostr-java-base/src/main/java/nostr/base/INostrList.java
nostr-java-event/src/main/java/nostr/event/list/BaseList.java
nostr-java-event/src/main/java/nostr/event/list/EventList.java
nostr-java-event/src/main/java/nostr/event/list/FiltersList.java
nostr-java-event/src/main/java/nostr/event/list/GenericTagQueryList.java
nostr-java-event/src/main/java/nostr/event/list/KindList.java
nostr-java-event/src/main/java/nostr/event/list/PublicKeyList.java

which have now been removed and all existing project classes (in particular, classes implementing IElement still do implement it) refactored as below, with all unit tests passing (and incidentally, my nostr-relay implementation, which relies heavily on these classes and their serializers/deserializers), functioning as expected:

before: EventList<GenericEvent> events;
   after: List<GenericEvent> events;

before: PublicKeyList<PublicKey> authors;
   after: List<PublicKey> authors;

before: KindList kinds;
   after: List<Kind> kinds;

before: EventList<GenericEvent> referencedEvents
   after: List<GenericEvent> referencedEvents;

before: PublicKeyList<PublicKey> referencePubKeys;
   after: List<PublicKey> referencePubKeys;

anyway, just wanted to bring the consideration to your attention and provide this PR as an alternate/"correct" implementation if you think there's value to it.

[avlo]

btw, upcoming encoder / decoder refactor is implemented atop changes submitted in this PR- so assuming this PR is accepted, an encoder / decoder PR will follow in step- which finalizes PR/submissions from me for the foreseeable future (unless otherwise needed/requested/etc). much thx for your time and accommodation 🙏

[tcheeric]

Understood, and thanks again. 🙏 -- Sent with Tuta; enjoy secure & ad-free emails: https://tuta.com 2 Jun 2024, 09:44 by ***@***.***:

…

btw, upcoming > "encoder / decoder refactor" <https://github.com/avlo/nostr-java-avlo-fork/tree/encoder_decoder-refactor>> branch is implemented atop changes submitted in this PR- so assuming this PR is accepted, the encoder / decoder branch will follow in step- which finalizes PR/submissions from me for the foreseeable future (unless otherwise needed/requested/etc). much thx for your time and accommodation 🙏 — Reply to this email directly, > view it on GitHub <#171 (comment)>> , or > unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQMG7DS2PKWEF5IXKN3G4DZFLLOVAVCNFSM6AAAAABIUSZG3SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBTG42TOMJSHA>> . You are receiving this because you are subscribed to this thread.> Message ID: > <tcheeric/nostr-java/pull/171/c2143757128> @> github> .> com>