Hi Oh

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Hi Oh

werner kassens-2
Hi Oh,
perhaps we could talk a bit about some other little problems i have with your contribution:
there are methods to make pharo compatibel to dolphin (eg SequenceableCollection>>writeStream). why should this be in a package for SciSmalltalk?

there are methods for eg morph that are not really necessary in a scientific library.

several methods just produce an error. eg try: #(1 2 1) union . quite a few methods are just inefficient reimplementations of methods in standard pharo. eg you could simply do #(1 2 1) union:#() instead of #(1 2 1) union.
another example:
integer>>copiesOf:  is a reimplementation of: Array new: withAll:
[400 copiesOf: 'r']bench." '8,340 per second.'"
[Array new: 400 withAll: 'r']bench." '131,000 per second.'"
or for example:
3333 primeFactorsByDaliot] bench. "'44,600 per second.'"
[3333 primeFactors asBag]bench. '51,800 per second.'

there is an big amount of methods added to object, behaviour, class and blockclosure. you probably dont know that many people consider this to be bad style and for example i dont want to use an image that does this. perhaps you might want to rethink some implemetations of methods that use these object- and behaviour methods.

i just looked at a few things here, but i guess you get the drift. to sum it up i'd think that it is not really necessary to load the whole project into sciSmalltalk and perhaps it would make sense if you load only StatPseudoCollections, what do you think?

best regards
werner

--
You received this message because you are subscribed to the Google Groups "SciSmalltalk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Hi Oh

HwaJong Oh
Thank you for reading my code and finding issues. I'm about look into the code, but before that I can agree that most of your point. It is caused because of the lazyness of keeping default-package named Daliot which is my nickname. I'm planing to remove this package completely but wont wondering why.

SequenceableCollection>>writeStream
Morph methods
Collection>>union
Integer>>copiesOf:
primeFactorsOfByDaliot
And all the other foreign methods added to Object class.

I'll try to inline or not use these method from the StatPseudoCollections side. Wait for the good news. :-)


By the way, off the topic, I want to advocate some of my codes:

#copiesOf: is not good for Value Objects. It is best when you use it on non value object.

100 copiesOf: aTetrisCell

which would have to be written in conventional way like

(1 to: 100) collect: [:neglected | aTetrisCell copy ]

I'll try not to use this method for StatPseudoCollections side.

And the #union is handy one liner when you have you have a collection of objects that answers collection but all you want is a union Set of them.

(myAmazonAccounts collect: [:each| each booksIBought]) union

I see no reason #union came with StatPseudoCollectionts. I'll try to not to use this, too.


#primeFactorsByDaliot was originally named #primeFactors, and it stayed for long time but Pharo 3 has implemented it with same name so I had to rename it to #primeFactorsByDaliot to avoid name collision. I was very impressed by the performance of Pharo 3's implementation of #primeFactors but I wanted to keep my old method for later study.


Once again, many of these issues will be gone when package Daliot is not loaded as prerequisit of StatPseudoCollections. Thank you for the review.

Best Regards,
HwaJong Oh


2014년 10월 5일 일요일 오후 9시 44분 18초 UTC+9, werner kassens 님의 말:
Hi Oh,
perhaps we could talk a bit about some other little problems i have with your contribution:
there are methods to make pharo compatibel to dolphin (eg SequenceableCollection>>writeStream). why should this be in a package for SciSmalltalk?

there are methods for eg morph that are not really necessary in a scientific library.

several methods just produce an error. eg try: #(1 2 1) union . quite a few methods are just inefficient reimplementations of methods in standard pharo. eg you could simply do #(1 2 1) union:#() instead of #(1 2 1) union.
another example:
integer>>copiesOf:  is a reimplementation of: Array new: withAll:
[400 copiesOf: 'r']bench." '8,340 per second.'"
[Array new: 400 withAll: 'r']bench." '131,000 per second.'"
or for example:
3333 primeFactorsByDaliot] bench. "'44,600 per second.'"
[3333 primeFactors asBag]bench. '51,800 per second.'

there is an big amount of methods added to object, behaviour, class and blockclosure. you probably dont know that many people consider this to be bad style and for example i dont want to use an image that does this. perhaps you might want to rethink some implemetations of methods that use these object- and behaviour methods.

i just looked at a few things here, but i guess you get the drift. to sum it up i'd think that it is not really necessary to load the whole project into sciSmalltalk and perhaps it would make sense if you load only StatPseudoCollections, what do you think?

best regards
werner

--
You received this message because you are subscribed to the Google Groups "SciSmalltalk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Hi Oh

werner kassens-2
Hi Oh,
thanks for taking care of these issues. i understand your points. it is just that copiesOf: is used in situations where one could use a simple new:withAll:. and instead of union one can in many situations eg use "flattened asSet" which, i agree, is not quite the same, but, i guess, could work in many cases.
werner

--
You received this message because you are subscribed to the Google Groups "SciSmalltalk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
For more options, visit https://groups.google.com/d/optout.