On Thu, Jan 30, 2014 at 2:32 PM, martin odersky <martin...@epfl.ch> wrote:

I was just trying out AnyRefMap. It's really nice to have a drop-in replacement for HashMap that avoids the "##" overhead that slows down the original HashMap. But I was frustrated that it's not a drop-in replacement. In particular I had several places where I inherited HashMap from an anonymous class or an object. Sometimes I needed to override the default value, at other times I wanted to add new behavior to the map. This works with HashMap (or any other map implementation), but not with AnyRefMap, because AnyRefMap is final. 

I think it would be great if we could eliminate this incoherence before 2.11RC1. What do others think?

On Thu, Jan 30, 2014 at 6:25 PM, Simon Ochsenreither <simon.och...@gmail.com> wrote:

I was just trying out AnyRefMap. It's really nice to have a drop-in replacement for HashMap that avoids the "##" overhead that slows down the original HashMap. But I was frustrated that it's not a drop-in replacement. In particular I had several places where I inherited HashMap from an anonymous class or an object. Sometimes I needed to override the default value, at other times I wanted to add new behavior to the map. This works with HashMap (or any other map implementation), but not with AnyRefMap, because AnyRefMap is final. 

I think it would be great if we could eliminate this incoherence before 2.11RC1. What do others think?

I'd consider any non-final collection class a severe bug.

 

Concerning inheriting from HashMap to override the default value ... what's wrong with using withDefault/withDefaultValue?

On Thu, Jan 30, 2014 at 7:53 PM, Rex Kerr <ich...@gmail.com> wrote:

 I agree that it's nice to have perfect drop-in replacements for things, but it's much harder to fashion a replacement that is a drop-in replacement for inheritance as well as for usage (composition).  The internals of HashMap and AnyRefMap are different, and although I have mimicked them as much as I could while retaining good performance, I chose not to allow inheritance from AnyRefMap in part _because_ I was worried that people would treat it as too much of a drop-in replacement for an altered HashMap.  In particular, some speedups come from code duplication (so you need to override in multiple places in AnyRefMap to change certain behavior); in some cases the call chain has reversed (e.g. which method do you need to override in order to add custom behavior to lookups?).

 

Overall, it seemed that there was no safe way to inherit broadly from AnyRefMap, and it was a lot easier to make the whole class final than figure out which methods were dangerous and which were safe.  (And don't forget that any code that assumes inheritance from HashTable is going to break also.)

With respect to default values, there are two problems with using default values that come from inheritance.  First, you can't propagate your defaults anywhere: you add one filter, and suddenly you lose your defaults. 

Secondly, they require you to have a non-final class in order to use them.  Even if we decided to make AnyRefMap non-final, someone else might want to inherit from it and make it final--and then they would lose the ability to set a default without specifically saying that was the desired behavior.

I do agree, however, that consistency is a considerable virtue.

So I am happy to change AnyRefMap to non-final, and I can also alter how defaults work, but both changes were made for good reason.  I suppose for aiding switching from HashMap to AnyRefMap, I could make AnyRefMap non-final but deprecate overriding with a note explaining how to achieve HashMap functionality with AnyRefMap without inheriting.

"AnyRefMap's implementation makes inheritance risky.  If you wish to set a default value, use the defaultEntry constructor parameter."

  --Rex

On Thu, Jan 30, 2014 at 5:32 AM, martin odersky <martin...@epfl.ch> wrote:

I was just trying out AnyRefMap. It's really nice to have a drop-in replacement for HashMap that avoids the "##" overhead that slows down the original HashMap. But I was frustrated that it's not a drop-in replacement. In particular I had several places where I inherited HashMap from an anonymous class or an object. Sometimes I needed to override the default value, at other times I wanted to add new behavior to the map. This works with HashMap (or any other map implementation), but not with AnyRefMap, because AnyRefMap is final. 

I think it would be great if we could eliminate this incoherence before 2.11RC1. What do others think?

Cheers

 - Martin

--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Thu, Jan 30, 2014 at 11:08 PM, Rex Kerr <ich...@gmail.com> wrote:

On Thu, Jan 30, 2014 at 2:03 PM, martin odersky <martin...@epfl.ch> wrote:

On Thu, Jan 30, 2014 at 6:25 PM, Simon Ochsenreither <simon.och...@gmail.com> wrote:

 

Concerning inheriting from HashMap to override the default value ... what's wrong with using withDefault/withDefaultValue?

An extra indirection. When I use AnyRefMap I do it to get the best performance. withDefault does not give me that.

In the case of AnyRefMap, this is irrelevant because you can supply a default mapping function in the constructor.

(I agree with the general point that surplus wrappers are bad for performance.)

  --Rex

 

--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Thu, Jan 30, 2014 at 11:31 PM, Simon Ochsenreither <simon.och...@gmail.com> wrote:

Why? Java's collections are non-final. What's wrong with that?

I'm not sure if this is a joke. Can you clarify?

Anyway, why should decisions made by people who built a different language 20 years ago have any impact on us?

An extra indirection. When I use AnyRefMap I do it to get the best performance. withDefault does not give me that.

Then it is probably the best to fix that indirection and move on.

--

You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

--

You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Thu, Jan 30, 2014 at 2:09 PM, martin odersky <martin...@epfl.ch> wrote:

On Thu, Jan 30, 2014 at 7:53 PM, Rex Kerr <ich...@gmail.com> wrote:

 I agree that it's nice to have perfect drop-in replacements for things, but it's much harder to fashion a replacement that is a drop-in replacement for inheritance as well as for usage (composition).  The internals of HashMap and AnyRefMap are different, and although I have mimicked them as much as I could while retaining good performance, I chose not to allow inheritance from AnyRefMap in part _because_ I was worried that people would treat it as too much of a drop-in replacement for an altered HashMap.  In particular, some speedups come from code duplication (so you need to override in multiple places in AnyRefMap to change certain behavior); in some cases the call chain has reversed (e.g. which method do you need to override in order to add custom behavior to lookups?).

Can you give outline which methods would not be safe to override? Do you expect that people would have a tendency to override anyone of these?

 

 

Overall, it seemed that there was no safe way to inherit broadly from AnyRefMap, and it was a lot easier to make the whole class final than figure out which methods were dangerous and which were safe.  (And don't forget that any code that assumes inheritance from HashTable is going to break also.)

With respect to default values, there are two problems with using default values that come from inheritance.  First, you can't propagate your defaults anywhere: you add one filter, and suddenly you lose your defaults. 

That's not a problem for typical use cases of mutable collections. You typically do not transform them, just use them in the standard CRUD way.

 

Secondly, they require you to have a non-final class in order to use them.  Even if we decided to make AnyRefMap non-final, someone else might want to inherit from it and make it final--and then they would lose the ability to set a default without specifically saying that was the desired behavior.

Well, I would say, that class then not be done in the Scala collection way. Overriding default is documented standard behavior. I do not see why we should break away now. And I particularly do not see why this should be for one, highly specialized class only. You just create an inconsistency without any gain that I can see. 

On Thu, Jan 30, 2014 at 11:53 PM, Rex Kerr <ich...@gmail.com> wrote:

On Thu, Jan 30, 2014 at 2:09 PM, martin odersky <martin...@epfl.ch> wrote:

On Thu, Jan 30, 2014 at 7:53 PM, Rex Kerr <ich...@gmail.com> wrote:

 I agree that it's nice to have perfect drop-in replacements for things, but it's much harder to fashion a replacement that is a drop-in replacement for inheritance as well as for usage (composition).  The internals of HashMap and AnyRefMap are different, and although I have mimicked them as much as I could while retaining good performance, I chose not to allow inheritance from AnyRefMap in part _because_ I was worried that people would treat it as too much of a drop-in replacement for an altered HashMap.  In particular, some speedups come from code duplication (so you need to override in multiple places in AnyRefMap to change certain behavior); in some cases the call chain has reversed (e.g. which method do you need to override in order to add custom behavior to lookups?).

Can you give outline which methods would not be safe to override? Do you expect that people would have a tendency to override anyone of these?

Any method where the call-chain isn't obvious is unsafe to override.  For example, HashMap has -= overridden 7 times and += 8 times in the library scan I did.  Update was overridden 0 times.  put was overridden 0 times.  Now, does += call update, or does update call +=, or neither call the other, or what?  In AnyRefMap, += calls update, but put has its own implementation.  In HashMap, update calls put, but += has its own implementation.  How is anyone supposed to keep this straight?

 

So in the wild we have 8 implementations of maps that are probably inconsistent because people didn't override all the necessary methods (and how could they have known?).

In my code, it's not safe to override much of anything that I implemented.  There's a fair bit of code duplication because removing the redundant logic manually provides better results than hoping the JIT compiler can do it.

 

In the Java HashMap code, it's safe to override things because there is only one way to do one thing in almost every case.

 

 

Overall, it seemed that there was no safe way to inherit broadly from AnyRefMap, and it was a lot easier to make the whole class final than figure out which methods were dangerous and which were safe.  (And don't forget that any code that assumes inheritance from HashTable is going to break also.)

With respect to default values, there are two problems with using default values that come from inheritance.  First, you can't propagate your defaults anywhere: you add one filter, and suddenly you lose your defaults. 

That's not a problem for typical use cases of mutable collections. You typically do not transform them, just use them in the standard CRUD way.

Maybe I'm atypical, but I do transform mutable collections.  Isn't allowing that the reason why they're in the same hierarchy as immutable collections?

I know (from bitter experience) that trying to extend anything you're going to transform is a losing battle, but sometimes that means I give up on the extension battle so I can transform.  (Sometimes I give up transformations so I can extend.)  If you use composition instead of inheritance for modifications, though, your extensions can survive reasonable transformations.

 

Secondly, they require you to have a non-final class in order to use them.  Even if we decided to make AnyRefMap non-final, someone else might want to inherit from it and make it final--and then they would lose the ability to set a default without specifically saying that was the desired behavior.

Well, I would say, that class then not be done in the Scala collection way. Overriding default is documented standard behavior. I do not see why we should break away now. And I particularly do not see why this should be for one, highly specialized class only. You just create an inconsistency without any gain that I can see. 

There is a gain, but it may well not be enough gain to warrant the inconsistency.  I grant that maintaining consistency is very important when possible.  For the record, I'm perfectly happy to go back to overriding and to mark every dangerous method as final (which is most of them).

  --Rex

 

--

You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

new AnyRefMap[String, String] with SynchronizedMap[String, String]

<console>:11: error: illegal inheritance from final class AnyRefMap

              new AnyRefMap[String, String] with SynchronizedMap[String, String]

So, we can't have a synchronized AnyRefMap?

If a deprecated, bit-rotted trait is the best use-case for keeping classes non-final, I'm delighted about the current approach of making everything final.

--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Thu, Jan 30, 2014 at 11:53 PM, Rex Kerr <ich...@gmail.com> wrote:

On Thu, Jan 30, 2014 at 2:09 PM, martin odersky <martin...@epfl.ch> wrote:

On Thu, Jan 30, 2014 at 7:53 PM, Rex Kerr <ich...@gmail.com> wrote:

 I agree that it's nice to have perfect drop-in replacements for things, but it's much harder to fashion a replacement that is a drop-in replacement for inheritance as well as for usage (composition).  The internals of HashMap and AnyRefMap are different, and although I have mimicked them as much as I could while retaining good performance, I chose not to allow inheritance from AnyRefMap in part _because_ I was worried that people would treat it as too much of a drop-in replacement for an altered HashMap.  In particular, some speedups come from code duplication (so you need to override in multiple places in AnyRefMap to change certain behavior); in some cases the call chain has reversed (e.g. which method do you need to override in order to add custom behavior to lookups?).

Can you give outline which methods would not be safe to override? Do you expect that people would have a tendency to override anyone of these?

Any method where the call-chain isn't obvious is unsafe to override.  For example, HashMap has -= overridden 7 times and += 8 times in the library scan I did.  Update was overridden 0 times.  put was overridden 0 times.  Now, does += call update, or does update call +=, or neither call the other, or what?  In AnyRefMap, += calls update, but put has its own implementation.  In HashMap, update calls put, but += has its own implementation.  How is anyone supposed to keep this straight?

 

So in the wild we have 8 implementations of maps that are probably inconsistent because people didn't override all the necessary methods (and how could they have known?).

In my code, it's not safe to override much of anything that I implemented.  There's a fair bit of code duplication because removing the redundant logic manually provides better results than hoping the JIT compiler can do it.

These are potential problems every Map implementation has. How many observed problems do we have in the JIRA database? 

 

In the Java HashMap code, it's safe to override things because there is only one way to do one thing in almost every case.

I don't believe that's 100% true. Does remove call get or put in the Java implementations? 

I do not want to argue that an inheritance based design is inevitable. Maybe a type-class based design or something else is better. We'd have to have one to experiment with to be able to tell for sure. What I do very strongly believe is that a design cannot and should not be gradually undermined and riddled with inconsistencies because we do not agree with its basic premises. Throw it out completely and replace it with a new one.  Or keep to it. That should be our only two options.

Cheers

 - Martin

 

 

Overall, it seemed that there was no safe way to inherit broadly from AnyRefMap, and it was a lot easier to make the whole class final than figure out which methods were dangerous and which were safe.  (And don't forget that any code that assumes inheritance from HashTable is going to break also.)

With respect to default values, there are two problems with using default values that come from inheritance.  First, you can't propagate your defaults anywhere: you add one filter, and suddenly you lose your defaults. 

That's not a problem for typical use cases of mutable collections. You typically do not transform them, just use them in the standard CRUD way.

Maybe I'm atypical, but I do transform mutable collections.  Isn't allowing that the reason why they're in the same hierarchy as immutable collections?

I know (from bitter experience) that trying to extend anything you're going to transform is a losing battle, but sometimes that means I give up on the extension battle so I can transform.  (Sometimes I give up transformations so I can extend.)  If you use composition instead of inheritance for modifications, though, your extensions can survive reasonable transformations.

 

Secondly, they require you to have a non-final class in order to use them.  Even if we decided to make AnyRefMap non-final, someone else might want to inherit from it and make it final--and then they would lose the ability to set a default without specifically saying that was the desired behavior.

Well, I would say, that class then not be done in the Scala collection way. Overriding default is documented standard behavior. I do not see why we should break away now. And I particularly do not see why this should be for one, highly specialized class only. You just create an inconsistency without any gain that I can see. 

There is a gain, but it may well not be enough gain to warrant the inconsistency.  I grant that maintaining consistency is very important when possible.  For the record, I'm perfectly happy to go back to overriding and to mark every dangerous method as final (which is most of them).

  --Rex

 

--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

riddled with inconsistencies

On Thu, Jan 30, 2014 at 3:13 PM, martin odersky <martin...@epfl.ch> wrote:

On Thu, Jan 30, 2014 at 11:53 PM, Rex Kerr <ich...@gmail.com> wrote:

On Thu, Jan 30, 2014 at 2:09 PM, martin odersky <martin...@epfl.ch> wrote:

On Thu, Jan 30, 2014 at 7:53 PM, Rex Kerr <ich...@gmail.com> wrote:

 I agree that it's nice to have perfect drop-in replacements for things, but it's much harder to fashion a replacement that is a drop-in replacement for inheritance as well as for usage (composition).  The internals of HashMap and AnyRefMap are different, and although I have mimicked them as much as I could while retaining good performance, I chose not to allow inheritance from AnyRefMap in part _because_ I was worried that people would treat it as too much of a drop-in replacement for an altered HashMap.  In particular, some speedups come from code duplication (so you need to override in multiple places in AnyRefMap to change certain behavior); in some cases the call chain has reversed (e.g. which method do you need to override in order to add custom behavior to lookups?).

Can you give outline which methods would not be safe to override? Do you expect that people would have a tendency to override anyone of these?

Any method where the call-chain isn't obvious is unsafe to override.  For example, HashMap has -= overridden 7 times and += 8 times in the library scan I did.  Update was overridden 0 times.  put was overridden 0 times.  Now, does += call update, or does update call +=, or neither call the other, or what?  In AnyRefMap, += calls update, but put has its own implementation.  In HashMap, update calls put, but += has its own implementation.  How is anyone supposed to keep this straight?

 

So in the wild we have 8 implementations of maps that are probably inconsistent because people didn't override all the necessary methods (and how could they have known?).

In my code, it's not safe to override much of anything that I implemented.  There's a fair bit of code duplication because removing the redundant logic manually provides better results than hoping the JIT compiler can do it.

These are potential problems every Map implementation has. How many observed problems do we have in the JIRA database?

 

 

In the Java HashMap code, it's safe to override things because there is only one way to do one thing in almost every case.

I don't believe that's 100% true. Does remove call get or put in the Java implementations?

 

I do not want to argue that an inheritance based design is inevitable. Maybe a type-class based design or something else is better. We'd have to have one to experiment with to be able to tell for sure. What I do very strongly believe is that a design cannot and should not be gradually undermined and riddled with inconsistencies because we do not agree with its basic premises. Throw it out completely and replace it with a new one.  Or keep to it. That should be our only two options.

On Fri, Jan 31, 2014 at 12:29 AM, Adriaan Moors <adr...@typesafe.com> wrote:

On Thu, Jan 30, 2014 at 3:13 PM, martin odersky <martin...@epfl.ch> wrote:

riddled with inconsistencies

Maybe we're talking about a different kind of consistency, but I don't see how making all leaf classes final is inconsistent.

It also fosters another kind of consistency: the internal behavior of the methods defined in these classes.

If you want to make your own collection, implement one of the interface traits, possibly with an underlying existing collection.

--

You received this message because you are subscribed to the Google Groups "scala-internals" group.

To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.

On Fri, Jan 31, 2014 at 12:37 AM, Jason Zaugg <jza...@gmail.com> wrote:

It would be better a wrapper like in java. Then it isn't susceptible to linearisation gotchas.  I looked at those a while back and found the impl to be a stark reminder of how much our mixins leave unprotected, for example toString, equals, serialisation. 

But in your example I would tend to write a wrapper manually that encapsulates the mutable unsynchronised map and offers a smaller API. It could contain a method to return an immutable snapshot if clients need rich query operations 

To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.

On Fri, Jan 31, 2014 at 12:37 AM, Adriaan Moors <adr...@typesafe.com> wrote:

On Thu, Jan 30, 2014 at 3:28 PM, martin odersky <martin...@epfl.ch> wrote:

I was not aware that SynchronizedMap is deprecated in 2.11.

It was based on this discussion: https://groups.google.com/d/msg/scala-internals/1gCbqTRCxAQ/7fJS6YsN50wJ

Assuming we need a good synchronized collection, while also agreeing "synchronization via mixin override trait is unnecessary",

the logical conclusion (to me) is deprecating the approach we don't like so that we can replace it by a better one.

We are making users go WTF all over the place with things that are broken like this (see also: delayedinit).

I say we should at the very least warn them, even if we don't have a better alternative. (The Java concurrent collections come to mind as an alternative.)

 

--

You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Fri, Jan 31, 2014 at 10:05 AM, Hanns Holger Rutz <con...@sciss.de> wrote:

+1

That was the whole point of things like `MapLike`--be able to put new

collections together by mixing in these traits.

On 31/01/2014 00:28, Rüdiger Klaehn wrote:
> I think using inheritance within the scala collections is fine. People
> contributing to scala know the pitfalls. But when using the scala
> collections from the outside people should not inherit from "leaf"
> collections. Inheriting from traits like Map/MapLike that are designed
> for inheritance is fine with some guidance in the docs.

--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Fri, Jan 31, 2014 at 11:48 AM, martin odersky <martin...@epfl.ch> wrote:

For all of these reasons I would say, let's try to work towards guideline (1), but do so in a way which is consistent over the whole collections. I do not see right now a better alternative for the `default` method. It does what it needs to do with minimum fuzz and performance overhead. So, I would suggest we make AnyRefMap non-final, but at the same time make as many of its methods final as we can. That will involve methods in its base classes, first, because a lot of the critical methods are probably inherited by AnyRefMap and second, because an isolated "improvement" for AnyRefMap would be a pessimization overall for breaking consistency.

 

2. The default method is clearly documented as a "hook", i.e. a documented extension point. And in fact I believe there's a good reason for that. Mainly, performance is the concern here. If one overrides default, the chances are that this is done so that instead of `get` one can call `apply` directly. So `default` is likely called on the most common access path for the collection. We simply do not want another closure object and an indirection for something like that.

The alternatives from Python do not apply, because Python has a completely different performance profile, and was not OO from the start.

 

3. Some people have said that MapLike and friends could replace the concrete Map classes as classes that are inherited. I am afraid that will not work. The ...Like classes are there to add utility methods such as map, filter, take to a base collection. ...Like classes do not contain a collection implementation themselves, they play essentially the role of containers of extension methods. So saying clients should extend MapLike instead of HashMap amounts to saying that they should write their own HashMap implementation from scratch!

 

For all of these reasons I would say, let's try to work towards guideline (1), but do so in a way which is consistent over the whole collections. I do not see right now a better alternative for the `default` method. It does what it needs to do with minimum fuzz and performance overhead. So, I would suggest we make AnyRefMap non-final, but at the same time make as many of its methods final as we can. That will involve methods in its base classes, first, because a lot of the critical methods are probably inherited by AnyRefMap and second, because an isolated "improvement" for AnyRefMap would be a pessimization overall for breaking consistency.

On Thu, Jan 30, 2014 at 2:03 PM, martin odersky <martin...@epfl.ch> wrote:

On Thu, Jan 30, 2014 at 6:25 PM, Simon Ochsenreither <simon.och...@gmail.com> wrote:

 

Concerning inheriting from HashMap to override the default value ... what's wrong with using withDefault/withDefaultValue?

An extra indirection. When I use AnyRefMap I do it to get the best performance. withDefault does not give me that.

In the case of AnyRefMap, this is irrelevant because you can supply a default mapping function in the constructor.

On Fri, Jan 31, 2014 at 12:08 PM, Rex Kerr <ich...@gmail.com> wrote:

On Fri, Jan 31, 2014 at 2:48 AM, martin odersky <martin...@epfl.ch> wrote:

Thanks for the feedback! There are too many mails to respond to them all, so let me try to summarize and explain my position.

1. Inheritance is a fundamental building block for Scala software  but it has to be used carefully (and I readily admit that the current collection could do with less, and would in fact be keen to see alternative implementations).

A good guideline is: "Every method should be abstract or final, or should be a documented extension point". In my mind that's a better guideline than "all leaf classes should be final". The latter has it backwards: Final classes are by necessity leaf-classes, but who is otherwise able to define what a leaf class is?

I like that guideline.  I think the existing classes are very very very far outside of that ideal, however.  Very little is documented as an extension point.

 

2. The default method is clearly documented as a "hook", i.e. a documented extension point. And in fact I believe there's a good reason for that. Mainly, performance is the concern here. If one overrides default, the chances are that this is done so that instead of `get` one can call `apply` directly. So `default` is likely called on the most common access path for the collection. We simply do not want another closure object and an indirection for something like that.

The alternatives from Python do not apply, because Python has a completely different performance profile, and was not OO from the start.

I benchmarked it both ways, and there is a small difference.  But emphasis on _small_.  The reason is that one extra indirection doesn't cost very much in heavily-used code, and lookup is already a pretty expensive operation.  I don't recall what the proportion was, but I'm pretty sure it was under 5%.

But I agree that it is clearly documented as a hook, and that has value.

 

3. Some people have said that MapLike and friends could replace the concrete Map classes as classes that are inherited. I am afraid that will not work. The ...Like classes are there to add utility methods such as map, filter, take to a base collection. ...Like classes do not contain a collection implementation themselves, they play essentially the role of containers of extension methods. So saying clients should extend MapLike instead of HashMap amounts to saying that they should write their own HashMap implementation from scratch!

Agreed.  MapLike does not give you a map.

 

For all of these reasons I would say, let's try to work towards guideline (1), but do so in a way which is consistent over the whole collections. I do not see right now a better alternative for the `default` method. It does what it needs to do with minimum fuzz and performance overhead. So, I would suggest we make AnyRefMap non-final, but at the same time make as many of its methods final as we can. That will involve methods in its base classes, first, because a lot of the critical methods are probably inherited by AnyRefMap and second, because an isolated "improvement" for AnyRefMap would be a pessimization overall for breaking consistency.

I recommend leaving other classes alone, mostly because it's a ton of work to wade through everything in base classes and think about whether each method can be sensibly overridden.  Usually the answer will be yes, after some deep thought, but yes-if-you-keep-these-things-in-mind.  And so that knowledge will have to be imparted to scaladoc.  I am quite sure I cannot get this done in anything like a comprehensive way before RC1, and right now AnyRefMap is an anomaly.

So I suggest instead that I _don't_ do anything to base classes (actually, there aren't any, only traits), since AnyRefMap's core functionality is all within AnyRefMap.  Of course it gets a ton of stuff from MapLike, but as you just said, MapLike doesn't implement a hash map. AnyRefMap does, and it's those internals that are hardest to get right.

And then we can gradually work towards better documentation of how to extend methods.

(Or, actually, I don't think that is ever going to happen in practice just through manual labor.  Instead, we need the compiler to tell us dependencies so the information is available automatically, so that when one is compiling, the compiler will warn you when you override too little to have a chance of making sense.)

  --Rex

--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On 30/01/2014 23:50, Adriaan Moors wrote:
> Making the leaf collection classes final will only make them more
> consistent,
> because we'll be fully in control of them, rather than having to
> anticipate all possible subclasses.

I agree with you. I also wonder... does it make sense at all to override
a leaf collection class if it's immutable? As soon as you modify
anything you would need also to make sure that you override `newBuilder`
and all that stuff.

On Fri, Jan 31, 2014 at 12:43 AM, martin odersky <martin...@epfl.ch> wrote:

On Fri, Jan 31, 2014 at 12:37 AM, Jason Zaugg <jza...@gmail.com> wrote:

It would be better a wrapper like in java. Then it isn't susceptible to linearisation gotchas.  I looked at those a while back and found the impl to be a stark reminder of how much our mixins leave unprotected, for example toString, equals, serialisation. 

Yes, that's a good point.  

But in your example I would tend to write a wrapper manually that encapsulates the mutable unsynchronised map and offers a smaller API. It could contain a method to return an immutable snapshot if clients need rich query operations 

I am not sure we can demand from users to write these wrappers that were so far created for them in the library. How about we throw together a SynchronizedWrapper for Map, then we can safely deprecate the old class? 

On Sat, Feb 1, 2014 at 3:02 AM, Peter Empen <peter...@arcor.de> wrote:

Please advice me on how to solve the following real-world requirement other than by inheritance:

About two years ago a client asked me to provide O(1) selection of random direct successors to a given node of a graph to enable him to implement random walks in an efficient manner. Until then successors were stored in collection.mutable.HashSet instances. I was able to use strait-forward inheritance to satisfy his request by adding a draw method: https://github.com/scala-graph/scala-graph/blob/master/core/src/main/scala/scalax/collection/mutable/ExtHashSet.scala.

 

To confirm, it is impossible to extend collection.imutable.HashSet mainly because the classes HashSet1, ..., HashTrieSet are placed in the companion object. What would be best practice to implement random selection in this case?

Please advice me on how to solve the following real-world requirement other than by inheritance:

About two years ago a client asked me to provide O(1) selection of random direct successors to a given node of a graph to enable him to implement random walks in an efficient manner. Until then successors were stored in collection.mutable.HashSet instances. I was able to use strait-forward inheritance to satisfy his request by adding a draw method: https://github.com/scala-graph/scala-graph/blob/master/core/src/main/scala/scalax/collection/mutable/ExtHashSet.scala.

 

To confirm, it is impossible to extend collection.imutable.HashSet mainly because the classes HashSet1, ..., HashTrieSet are placed in the companion object. What would be best practice to implement random selection in this case?

For another reason, I recently also tried to extend java.util.IdentityHashMap but I stranded due to package visibility and lots of private members. So I had to "convert" it to Scala (EqHashMap in the same folder).

I believe if you decide to seal collection classes we do need a means that allows users to reuse collection implementations in some way. For instance by abstract base classes as indicated by Jason and others.

On Sat, Feb 1, 2014 at 12:02 PM, Peter Empen <peter...@arcor.de> wrote:

--

You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-i...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.