Rename connect to join

[barosl]

Kawaii version

Related Rust issue: rust-lang/rust#24645

[gkoz]

👍
Had to ask on irc to find out that join is called connect.

[gsingh93]

+1

[mdinger]

s/gurantee/guarantee/

[quantheory]

I approve, though I don't have a strong opinion. I do want to point out that if stability is an issue, we can have two names for the same function, and deprecate the older one. As I understand it, the core team has decided that adding methods without changing/removing existing methods is considered backwards compatible, even though there's a small chance of a name clash when adding a method. Not allowing methods to be added would be just too restrictive, since the standard library would really not be able to evolve its existing types.

[tshepang]

👍

[tshepang]

I'd go as far as getting rid of concat, in favor of join(""). It feels like overkill to have a whole function just for that special case. I thought the advantage would be performance, but not even. Following:

#![feature(test)]
extern crate test;

#[bench]
fn connect(b: &mut test::Bencher) {
    b.iter(|| {
        ["foo", "bar", "baz", "qux"].connect(" ")
    });
}

#[bench]
fn concat(b: &mut test::Bencher) {
    b.iter(|| {
        ["foo", "bar", "baz", "qux"].concat()
    });
}

Gives:

test concat  ... bench:        82 ns/iter (+/- 14)
test connect ... bench:        79 ns/iter (+/- 7)

[andrew-d]

👍 from me - this was something I had to look up too.

[gsingh93]

If we had done this earlier, deprecating/removing concat might have been fine, but I don't know if its worth doing it now. I don't have a strong opinion either way.

[bluss]

Full support if we can deprecate the old names before 1.0, otherwise I think it's a silly change. We can live with a difference like this.

[tshepang]

Why not deprecate even after 1.0 @bluss?

[bluss]

I'm not entirely against it, it just seems we could live with the difference instead. The name is not exactly monumental mistake. The positive sides are that deprecation is easy and the warning message should mean that almost everyone uses the non-deprecated name, but it's still another extra entry in the docs.

[tshepang]

@bluss a core team member is planning on having docs for deprecated items hidden by default

[gsingh93]

Since the trait is marked as unstable, will any code currently building with the beta actually break if the methods are removed?

[bluss]

Yes, because the methods are callable in beta (playpen link)

fn main() {
    println!("{}", ["foo", "bar", "baz", "qux"].connect(" "));
}

[barosl]

@gsingh93 While the trait is unstable, the stable methods are already exported to the prelude. That's the problem!

[ArtemGr]

👍 I've searched the docs for join a few times and came away empty-handed. Turns out it's there, but under an unexpected name. Let's maintain the principle of least astonishment.

[tanadeau]

👍 I also had a lot of trouble finding this.

[robinst]

👍 was also looking for this under the name "join".

[aturon]

The libs team has been working through the RFC backlog, and I wanted to restart discussion on this RFC.

Unfortunately, since 1.0 has shipped, the calculus here is perhaps a bit different than during the original comment period. In particular, since the connect method is stable, this would require a deprecation, along the lines discussed in this internals thread.

While I'm definitely sympathetic to the idea that matching existing precedent is good, I also feel that connect is a reasonable name (in some ways more evocative than join), and given that this would require a deprecation, I'm not personally sure it's worth it.

[Gankra]

This seems like another great candidate for something like
#1150

On Thu, Jun 11, 2015 at 10:33 AM, Aaron Turon notifications@github.com
wrote:

The libs team has been working through the RFC backlog, and I wanted to
restart discussion on this RFC.

Unfortunately, since 1.0 has shipped, the calculus here is perhaps a bit
different than during the original comment period. In particular, since the
connect method is stable, this would require a deprecation, along the
lines discussed in this internals thread
https://internals.rust-lang.org/t/thoughts-on-aggressive-deprecation-in-libstd/2176/
.

While I'm definitely sympathetic to the idea that matching existing
precedent is good, I also feel that connect is a reasonable name (in some
ways more evocative than join), and given that this would require a
deprecation, I'm not personally sure it's worth it.

—
Reply to this email directly or view it on GitHub
#1102 (comment).

[nagisa]

I suggest interpolate, intercalate and intersperse as alternative renamings.

[ssokolow]

The problem with connect is that, in a programming context, the verb has built up an implicit association with IPC, which might lead to an effect similar to banner blindness where people keep missing it because they "know" that's not what they're looking for on a level too low to realize that IPC support in strings makes no sense.

[alexcrichton]

This RFC is now entering its week-long final comment period.

[DanielKeep]

👍 This bit me three times until I could finally remember what Rust called this method (the second and third times, all I could remember was "it's called something stupid which I expect to mean something else" and not the name).

This is a nasty newbie trap; I can't think of any good reason to not at least introduce join as a synonym.

[rkjnsn]

👍 for join. I think it is worth the deprecation.

[rtkrruvinskiy]

👍 When first trying Rust, I spent a long time looking for this functionality under join and in the end failed to find anything. I eventually asked and was pointed to connect. Never in a million years would I have guessed the name.

I can definitely understand the reluctance to deprecate, but I think join should be a synonym even if connect stays.

[sfackler]

I am somewhat confused as to why this method is defined on slices in the first place as opposed to iterators. If I want to project strings out of a collection of structs and then join them, I'd currently have to collect them into a Vec first which seems unfortunate/unnecessary.

I'd like to see a proposal for the addition of a join API to Iterator (or maybe an extension trait?). I'm not sure if the basic signature that exists now is the right way to go either - should it return String/Vec<T> directly? Should it be passed a &mut String/&mut Vec<T> like some of the Read methods do? Should it be able to take a Writer to potentially avoid intermediate allocation?

EDIT: If the API moves to Iterator it might still be worth adding convenience methods to String and Vec, however.

[bluss]

@sfackler When slices already are present, it's a great advantage to use that, because the implementations walk them twice, once to compute the exact space needed and once to do the concatenation.

When the slice is already present this is much superior, and it might even be more performant even when including a collect, depending on the number and length of the strings.

So that's the benefit of slices, that doesn't preclude a method on iterators. We absolutely should opt for consistency though — I'm in favour of .connect() on an iterator if that's the name that stays.

[gkoz]

@bluss I don't see why it has to be a slice if a clonable iterator is sufficient.

[tafia]

I'm used to join in other languages too. connect make me think of more complicated operation (e.g. databases).

[jminer]

👍 If I were looking for this method, I would look for join. I don't know if I would find it with its current name.

[Veedrac]

I'm in favour of join on cloneable iterators and deprecating connect. I don't see the downside of using cloneable iterators and join is a far better name.

[Ryman]

@Veedrac I think the main downside currently is that rust-lang/rust#23501 is not implemented, so much of current iterator usage isn't currently cloneable?

[Veedrac]

@Ryman That's not much of a problem, since it still leaves us in a strictly better situation whilst we're waiting.

[sfackler]

Why does the iterator need to be clonable? Being able to allocate the space up front does not seem to be sufficiently amazing to forbid join's use with non-cloneable iterators. We can always add an optimized version when specialization or something equivalent is implemented.

[nstoddard]

👍

[devonhollowood]

I'm fine with adding join as a synonym, but getting rid of connect just doesn't seem worth it to me. I don't think the issue is big enough to warrant deprecation.

[alexcrichton]

The libs team did not reach consensus on this issue during this past FCP. To help focus the discussion though we think that the question of what this method is defined on (slices or iterators) is orthogonal to what it's called, so a future RFC can tweak this design but that's not quite the focus of this RFC.

Additionally it's probably not worth blocking this RFC on waiting for a system for renaming methods, so the only major objection to this is that it may not be worth the cost that it incurs (e.g. deprecating a method, straddling versions is harder, etc). The support here may be enough to outweigh this, however.

As a result we're going to leave this RFC in the FCP for another week.

[adwhit]

👍 Got bitten by this yesterday!

[yongqli]

👍

[leoyvens]

👍

[lambda]

👍 for join, and deprecating connect. May be a good idea to do add the new alias, wait some time, and then deprecate the old one, if you want to avoid the version compatibility issues that can come from immediately deprecating (lots of people switch all at once to silence the warnings, but then none of that code runs on old releases that may be present in various package managers). However, I think it's still early enough, and there are few enough (if any) platforms in which Rust is packaged in any kind of long-term supported release, that it's probably still fine to do immediate deprecation.

[nathankleyn]

👍 I agree with @lambda in regards to an eventual deprecation after a period of a stable alias. This bit me when I first tried rust as I was so used to it being called join in virtually every other language.

[alexcrichton]

After this next week of FCP the libs team is now in consensus to merge this RFC, so I shall do so. Thanks again for the discussion everyone and for the RFC @barosl!

Tracking issue

[tshepang]

\0/