Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Char#code_point method #6035

Closed
wants to merge 1 commit into from
Closed

Add Char#code_point method #6035

wants to merge 1 commit into from

Conversation

esse
Copy link
Contributor

@esse esse commented Apr 30, 2018

When we use Char, often use case is to get it's code point.

However in Crystal there isn't code_point method on Char, only ord. This PR adds code_point method (which is intuitive), that returns code point of given char (same as ord).

I've included specs and documentation for this method.

@ysbaddaden
Copy link
Contributor

We have a no-aliases convention in Crystal, so we can't have both ord and code_point.

@esse
Copy link
Contributor Author

esse commented Apr 30, 2018

@ysbaddaden Hm, I looked into source for Char and found for example:

  # Same as `to_i`.
  def to_i32(base : Int = 10) : Int32
    to_i(base)
  end

or other aliases, like (in iterable.cr):

  # Same as `each.cycle`.
  def cycle
    each.cycle
  end

Is this a new convention or...?

@z64
Copy link
Contributor

z64 commented Apr 30, 2018

Those aren't aliases.

class Foo
  def name_foo
    # ...
  end

  def name_bar
    name_foo
  end
end

An alias serves no other purpose but to provide an alternate name for a single function.

The methods you show call different functions / wrap / shortcut to other behavior, not just "rename" a method.

@ysbaddaden
Copy link
Contributor

ysbaddaden commented Apr 30, 2018

@straight-shoota
Copy link
Member

We don't want aliases but it'd still be a valid proposal to rename ord to code_point. It's at least more descriptive.

On the other hand ord (and char) are traditional names for this conversion and established in many programming languages, including Crystal.

@esse
Copy link
Contributor Author

esse commented Apr 30, 2018

I think in some rare cases, aliases are useful - such example would be TempFile:
https://github.com/crystal-lang/crystal/blob/master/src/tempfile.cr#L110

  def unlink
    delete
  end

Here unlink is an alias to delete.

I believe that we should keep ord (it's used that way in Ruby, and it's often used in languages) and add the alias of code_point.

code_point is very verbose, therefore rendering mentioned comment:

more aliases mean more names to learn. Even if you don't use an alias, if somebody on your team or a project you are working on uses it, you'll have to learn it.

unapplicable here. I believe that when you want to get code point of char, the fact that it can be get using method code_point doesn't occupy any space in mind and doesn't give any mental overhead. On the other hand, if I use ord, I still know that it returns code_point. So I believe it's still worth adding (same as TempFile#delete and TempFile#unlink).

@ysbaddaden
Copy link
Contributor

Tempfile#unlink is a lost alias. It should be removed because the proper naming is delete (e.g. there is File#delete but not File#unlink). There may be more lost aliases that should be removed.

If we keep Char#ord then we won't introduce Char#code_point. I have little opinion over which is better, thought I would recognize #ord but wouldn't be sure what #code_point actually is.

@esse
Copy link
Contributor Author

esse commented Apr 30, 2018

I understand - the crucial point of information here is that all existing aliases are legacy code.

I'm closing this PR, and I would be thrilled to help hunt these lost aliases down - will be making PRs if I found any (like #6036).

@esse esse closed this Apr 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants