Make Snowflake -> String conversion more idiomatic

[BenWoodworth]

This PR deprecates Snowflake.asString, and gives its functionality to Snowflake.toString() instead.
(And migrates all usages of .asString in the repo to .toString())

With this change, "$snowflake" can be done instead of "${snowflake.asString}".

The main changes are in Snowflake.kt, with all the other changes being the migrations.

---

Thinking this is more idiomatic is mostly based on intuition, but I have some rationale:

• Can't think of a use case needing the string "Snowflake(value=...)" outside of println debugging
  • Snowflake.toString() is never used in this repo
  • "...${snowflake.asString}..." is used 26 times, when "...$snowflake..." would be more convenient
• asString being a property feels out of place, since semantically it's a conversion, not an attribute of Discord snowflakes
  • Although I can think of kotlinx libraries that use properties for convenience conversions (e.g. JSON)
• I expect symmetry when a value type has a String constructor: snowflake == Snowflake(snowflake.toString())
• I expect symmetry between .to*() and .as*() functions: snowflake.asString == snowflake.toString()

[HopeBaron]

I've to mention that before we proceed that asString doesn't have the same functionality as toString as your PR gives the impression you thing they do.
asString gives the id value in the form of a String, such as: "908077612016992316"
where as toString shows "Snowflake(908077612016992316)"
that being cleared up, what makes this PR valid.
you can't compare json retrieved values with toString since Snowflake(908077612016992316) != 908077612016992316 (assuming strings)

[BenWoodworth]

The PR changes the functionality toString() to be the same as asString: diff

So that means you can compare json retrieved values with toString, since
Snowflake(908077612016992316).toString() == "908077612016992316"

Is that what you mean?

[HopeBaron]

But this is not a string representation of a snowflake
it could be of any long number, no citation is included in this representation

[BenWoodworth]

I think I get what you're saying. Is there a reason the citation's needed in the String representation?

It's normal in idiomatic Kotlin for toString() to be indistinguishable from other types (e.g. 1, 1uL, "1")
This also is the representation used by Discord's REST APIs (since snowflakes are integers, just with special semantics)

[HopeBaron]

But they are numbers still, just limited, Snowflakes are not normal numbers in the system, they hold things like timestamps, date of creation etc.

[BenWoodworth]

Yep! That's the semantics I was talking about

[HopeBaron]

I think I was also unclear in the previous comment comment.
You said

It's normal in idiomatic Kotlin for toString() to be indistinguishable from other types (e.g. 1, 1uL, "1")

but these are numbers, when you read the value you know it's a number, however, when you read a number that belongs to s a snowflake, you have no clue it's a snowflake.

Snowflakes are not just numerical numbers, they hold, again, data like when the entity was created.
So it makes no sense to make a representation for snowflakes that give the impression that they are just numerical values.

Thus this is why asString exist. to allow get the string value of the snowflake, where as toString is for the representation

[BenWoodworth]

Yeah I completely agree, but I'm not saying the string representation should give the impression that it's just a number. That wasn't really my point

This is the general rule for toString() that you'll find in idiomatic Kotlin:

• If a type has a useful string representation, return that for toString()
• Otherwise, return something generic that textually represents it
  • "SomeClass(a=7)" is obviously a lot better than "SomeClass@4cb2c100"

This is how you'll find it done everywhere in the official Kotlin libraries. Like Regex("1234").toString() is "1234". That's a lot more useful to developers than "Regex(pattern=1234)", even though there's nothing to indicate that "1234" is a regex.

I think you could agree that for Snowflake, "1111" is a lot more useful to programmers than "Snowflake(value=1111)" when writing code. The fact that Snowflake.asString is used 86 times in this repo while Snowflake.toString() isn't even used once. (besides in other toString() methods)

There is a good reason why toString() is done like this. It's much nicer to write...

CdnUrl("$BASE_URL/guilds/$guildId/users/$userId/avatars/$hash") // this
CdnUrl("$BASE_URL/guilds/${guildId.asString}/users/${userId.asString}/avatars/$hash") // than this

I'm also not sure I understand the concern with not knowing the number is a snowflake. E.g. "GuildBehavior(id=1111)" seems okay the same way {id: "1111", ...} from the API, clicking Copy ID in discord and getting "1111" in my clipboard, and Regex("1234").toString() == "1234" are okay. If that makes sense...

[BenWoodworth]

And sorry for the wall of text. I hope I'm not being annoying because I don't mean to be, I just want to flesh out my thoughts. I've been using Kotlin as my language of choice since it was in beta and have a firm grasp on its conventions. I'm loving Kord so far and want to nudge it in the right direction if can, helping iron out any awkward or non-idiomatic APIs. Don't stress out about this PR if you really don't want to include the change :)

[HopeBaron]

Hey Ben, sorry for the late reply.
I appreciate your help on this. Also thanks for bringing this up.

I looked that up but I didn't find any source that mention such a convension. could you link me to the source?

[BenWoodworth]

Hah no worries, and good question! I don't think it's documented anywhere as a hard-and-fast rule, that's just how I see it done in the official libraries. Like there aren't right/wrong ways to implement toString(), just more/less useful ways

[HopeBaron]

Hey Ben, sorry for not responding, Your points are valid and I don't see a reason to keep the toString as is, users can always define their own logging string on the entities. Tho I wonder how would this affect logging

[BenWoodworth]

Hmm, you'd know better than I do since I haven't paid attention to Kord's logging at all so far

[HopeBaron]

Will merge this, we just have to wait for Scheduled events and auto complete to be merged.