metaeducation / rebol-issues

6 stars 1 forks source link

A refinement for CHANGE #1570

Open rebolbot opened 14 years ago

rebolbot commented 14 years ago

Submitted by: Ladislav

As opposed to the INSERT native, where the /PART refinement applies to the second argument (see the code below), the same CHANGE refinement applies to its first argument.

This is misleading REBOL users, needs a lot of "adjustments", and even long-time REBOL users are still making mistakes in it, as I noticed just recently (myself not being an exception).

An even greater problem is the fact that a corresponding refinement being applied to the second argument of CHANGE does not exist at all. This makes implementations of algorithms needing such a refinement so inefficient that the users "routinely" prefer fundamentally inefficient (O(N \ 2)) algorithms is such cases.

Not asking to "break R2 compatibility" so that the /part would apply to the second argument of the CHANGE function and wanting to at least "cure" the greatest problem, I propose to introduce a new refinement with a behaviour analogical to the /part refinement of the INSERT function, i.e., a refinement that would be applied to the second argument of the CHANGE function.

I am not the best at picking good names, leaving that part to the discussion.

; the /part refinement of INSERT is applied to its second argument:
>> head insert/part "123" "456" 2
== "45123"

; the /part refinement of CHANGE is applied to its first argument:
>> head change/part "123" "456" 2
== "4563"

CC - Data [ Version: alpha 97 Type: Wish Platform: All Category: Native Reproduce: Always Fixed-in:none ]

rebolbot commented 14 years ago

Submitted by: BrianH

It's weird, I always thought of CHANGE's behavior as the normal way to use /part and INSERT's way to be the strange one. To each their own, I guess.

Breaking backwards compatibility by changing which argument /part refers to would probably be a bad thing in this case, because the change may be too stealthy for coders used to R2 to pick up on, leading to hard-to-find bugs. We try to not change basic functionality like this without renaming the function or option, at least where the semantics of the underlying data model haven't been drastically changed like they were with ports, graphics and objects. That is definitely not the case here: Changing the argument that /part refers to would not be a side-effect of the fix in the off-by-one error for negative indexes, or protected series, and there haven't been any other major semantic changes to series semantics. But if we decide that it's a change worth making, I'd have no problem with it either way.

As for the second option, to apply to the other argument, that would be a good idea. But /by is probably not the best name for it. Some suggestions from the AltME discussion about the subject: /span, /range, /to, /from, /interval. I like /span.

rebolbot commented 14 years ago

Submitted by: Ladislav

"I always thought of CHANGE's behavior as the normal way to use /part and INSERT's way to be the strange one"

Then you are probably an exception. My records indicate that nobody is confused by INSERT/PART being related to the second argument, while I found many users (including myself) confused by CHANGE/PART being related to the first argument.

rebolbot commented 14 years ago

Submitted by: BrianH

"then you are probably an exception"

Unsurprising. I used CHANGE/part for years before I first used INSERT/part; it just never came up in practice at first.

There's no sensible meaning to having /part refer to the first argument of INSERT, or more recently APPEND and REPEND. As for CHANGE/part, it could go either way. The name /span to refer to the other argument would make slightly more sense if /part referred to the second imo, so that and consistency with INSERT and APPEND would be good arguments in favor of the change.

rebolbot commented 14 years ago

Submitted by: Gregg

I think the current behavior is something to state clearly in docs, and Ladislav's example make the behavior very clear. My perspective is more like Brian's in this case, as to what I use more and think of as "normal". It's certainly tricky, but I don't know if a new refinement will help or make things more confusing.

In a quick search, I only found INSERT/PART used by a small number of scripts, and in all cases, the refinement arg is a series offset, not an integer. That makes sense to me. In that context, do you think we still need a new refinement?

rebolbot commented 14 years ago

Submitted by: Ladislav

"...the refinement arg is a series offset, not an integer"

By "a series offset" I understand "a series, having the same head as the second argument, but different index". That is exactly the case which I expect to be the most frequent for the proposed "second-argument-influencing" CHANGE refinement too

rebolbot commented 14 years ago

Submitted by: BrianH

I agree with Ladislav here: The /span option should behave like the /part option, but apply to the other argument. And barring compatibility issues (perhaps related to #666) it would be best to have /part refer to the second argument and /span refer to the first, imo.

Nice pun in the summary, btw :)

rebolbot commented 14 years ago

Submitted by: Ladislav

I want to mention Christian's idea that the ideal refinement name should suggest whether it relates to the first or second argument. (although it is not easy to invent)

rebolbot commented 10 years ago

Submitted by: abolka

Also see #2096 discussing the expected behaviour for APPEND/part and INSERT/part.