You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@jena.apache.org by GitBox <gi...@apache.org> on 2022/08/30 12:18:44 UTC

[GitHub] [jena] Aklakan commented on issue #1498: Prologue not compatible with PrefixMapping2

Aklakan commented on issue #1498:
URL: https://github.com/apache/jena/issues/1498#issuecomment-1231588129

   Hm, the advantage of PrefixMapping2 is that the internal 'added' map is again a PrefixMapping and the lookup inherts its performance characteristics.
   
   BufferingPrefixMap only uses a plain HashMap for the additions which means iri-to-prefix lookups fall back to O(n)
   I guess in the presence of deletions there is not really a better way.
   
   So BufferingPrefixMap is not a replacement from a performance perspective. 
   
   > One-impl-does-everything can be hard to maintain.
   Yes, but
   - most importantly, I'd expect that in general use a solution that can deliver O(log(n)) is appreciated over a O(n) one and
   - implementations are not *that* complex and
   -  the existence of PrefixMapping2 (and the removed FastAbbreviatingPrefixMap) testifies that at least at some point these use case were relevant (and they *still* are for me).
   
   I would also be fine with doing a port of PrefixMapping2 to e.g. PrefixMap2. But it's not exhilarating having to clutter my own projects with such rather basic stuff (especially when it already existed).
   
   And actually the problem is not even related to PrefixMapping2 but a needless call to removeNsPrefix in Prologue that would break any PrefixMapping2-like approach:
   
   ```java
   // class Prologue:
               String oldExpansion = prefixMap.getNsPrefixURI(prefix) ;
               if ( Objects.equals(oldExpansion, expansion) )
                   return ;
               if ( oldExpansion != null ) // <- remove
                   prefixMap.removeNsPrefix(prefix) ; // <- remove
   
               prefixMap.setNsPrefix(prefix, expansion) ; // Isn't the contract of PrefixMap anyway that this should replace a prior mapping?
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@jena.apache.org
For additional commands, e-mail: issues-help@jena.apache.org