translateBy: vs translatedBy:

On Tue, May 26, 2015 at 9:38 PM, Peter Uhnák <[hidden email]> wrote:
>> I vote for renaming to Rectangle>>translatedBy:, because it returns a new
>> translated rectangle instance.
>
>
> -1
> There is no benefit in renaming a single method, apart from creating more
> confusion.

I wouldn't say "no benefit".  Consistent conventions are beneficial.
It could be useful to bring these into line with the general "-ed"
convention of returning a new object.      I don't see how this would
create confusion, though it might create work.  Whether its worth the
work is a slightly different question.

> If anything, the whole Rectangle API would have to be changed, as there is
> plethora of similar methods (expandBy:, extendBy:, merge:, etc.)

For a better feel of the work involved...

Rectangle methods select: [ :m | m sourceCode includesSubstring: '^
Rectangle ' ].
    Rectangle>>#intersect:
    Rectangle>>#quickMerge:
    Rectangle>>#encompass:
    Rectangle>>#intersect:ifNone:

Rectangle methods select: [ :m | m sourceCode includesSubstring:
'^Rectangle ' ].
    Rectangle>>#intersect:
    Rectangle>>#translateBy:
    Rectangle>>#scaleBy:
    Rectangle>>#merge:
    Rectangle>>#compressTo:
    Rectangle>>#expandTo:
    Rectangle>>#roundTo:
    Rectangle>>#truncateTo:

The following already follow the convention...
    Rectangle>>#compressed
    Rectangle>>#expanded
    Rectangle>>#rounded
    Rectangle>>#truncated

> Pretty much all Rectangle messages are immutable and create new Rectangle,
> thus I feel it is unnecessary to state it explicitly.
>
> In VW there is plenty of methods (e.g. left:) that mutates it, thus unlike
> Pharo a clear distinction is required.

Still, it is reasonable to consider the impact of inconsistent
conventions on the rest of the system outside of Rectangle.

cheers -ben

>
> On Tue, May 26, 2015 at 6:14 PM, Ben Coman <[hidden email]> wrote:
>>
>> On Tue, May 26, 2015 at 9:38 PM, Peter Uhnák <[hidden email]> wrote:
>> >> I vote for renaming to Rectangle>>translatedBy:, because it returns a
>> >> new
>> >> translated rectangle instance.
>> >
>> >
>> > -1
>> > There is no benefit in renaming a single method, apart from creating
>> > more
>> > confusion.
>>
>> I wouldn't say "no benefit".  Consistent conventions are beneficial.
>> It could be useful to bring these into line with the general "-ed"
>> convention of returning a new object.      I don't see how this would
>> create confusion, though it might create work.  Whether its worth the
>> work is a slightly different question.
>>
>> > If anything, the whole Rectangle API would have to be changed, as there
>> > is
>> > plethora of similar methods (expandBy:, extendBy:, merge:, etc.)
>>
>> For a better feel of the work involved...
>>
>> Rectangle methods select: [ :m | m sourceCode includesSubstring: '^
>> Rectangle ' ].
>>     Rectangle>>#intersect:
>>     Rectangle>>#quickMerge:
>>     Rectangle>>#encompass:
>>     Rectangle>>#intersect:ifNone:
>>
>> Rectangle methods select: [ :m | m sourceCode includesSubstring:
>> '^Rectangle ' ].
>>     Rectangle>>#intersect:
>>     Rectangle>>#translateBy:
>>     Rectangle>>#scaleBy:
>>     Rectangle>>#merge:
>>     Rectangle>>#compressTo:
>>     Rectangle>>#expandTo:
>>     Rectangle>>#roundTo:
>>     Rectangle>>#truncateTo:
>>
>> The following already follow the convention...
>>     Rectangle>>#compressed
>>     Rectangle>>#expanded
>>     Rectangle>>#rounded
>>     Rectangle>>#truncated
>>
>> > Pretty much all Rectangle messages are immutable and create new
>> > Rectangle,
>> > thus I feel it is unnecessary to state it explicitly.
>> >
>> > In VW there is plenty of methods (e.g. left:) that mutates it, thus
>> > unlike
>> > Pharo a clear distinction is required.
>>
>> Still, it is reasonable to consider the impact of inconsistent
>> conventions on the rest of the system outside of Rectangle.
>>
>> cheers -ben
>>
>

On Wed, May 27, 2015 at 1:16 AM, Peter Uhnák <[hidden email]> wrote:
> That's not all, many methods return new rectangles via Point's API, namely
> Point>>{#corner: #extent: #rectangle:}
>
> I don't know if I can perform static analysis of the code (are there tools
> for that in Pharo?) to see which methods exactly return Rectangle
>
> rectMethods := #(#corner: #extent: #rectangle:).
> (Point methods select: [ :m | rectMethods includes: m selector ]
> thenCollect: [ :m | m senders ]) flattened select: [ :s | s className =
> #Rectangle ]. "an Array(
> Rectangle>>#areasOutside:
> Rectangle>>#withSideOrCorner:setToPoint:minExtent:limit:
> Rectangle>>#withBottom:
> Rectangle>>#bottom:
> Rectangle>>#areasOverlapingOutside:
> Rectangle>>#floor
> Rectangle>>#allAreasOutsideList:startingAt:do:
> Rectangle>>#scaledAndCenteredIn:
> Rectangle>>#squishedWithin:
> Rectangle>>#ceiling
> Rectangle>>#withHeight:
> Rectangle>>#rectanglesAt:height:
> Rectangle>>#scaleFrom:to:
> Rectangle>>#withLeft:
> Rectangle>>#left:
> Rectangle>>#withTop:
> Rectangle>>#right:
> Rectangle>>#top:
> Rectangle>>#innerCorners
> Rectangle>>#interpolateTo:at:
> Rectangle>>#withWidth:
> Rectangle>>#quickMergePoint:
> Rectangle>>#withRight:
> Rectangle>>#rotateBy:centerAt:
> Rectangle>>#withSideOrCorner:setToPoint:minExtent:limit:
> Rectangle>>#flipBy:centerAt:)
> "
>
> This list is maybe a bit excessive (sometimes it returns self, sometimes new
> rectangle -- e.g. floor).
>

Good   Point   ;)

However the "-ed" convention doesn't make sense for many of those...
    #areasdOutside: , #areasedOutside , #areaOutsided:  ?
... and #areasOutside is obviously a different rectangle than the original.

The scope might be restricted to only those where the usual convention
makes the names misleading, for example:
   #merge

cheers -ben