Add Armstrong Numbers exercise #893

Tuxified · 2018-10-31T22:52:17Z

As this is my first stab at a Ruby exercise I didn't use the test generator, hope that's ok.

The solution is not using Integer.digits as that is available on Ruby 2.4, not previous ones.
Let me know what you think :)

TODO:

update config.json

As for the config .json metadata, difficulty 1 or 2, categories: integers, loops ?

- not using Integer.digits as that is only available on Ruby 2.4

complying with name convention

Insti · 2018-11-01T18:17:43Z

Great. thanks!
@F3PiX can you give your estimation of difficulty and where this should fit in the tree?

exercises/armstrong-numbers/.meta/solutions/armstrong_numbers.rb

exercises/armstrong-numbers/armstrong_numbers_test.rb

Insti · 2018-11-01T18:23:50Z

exercises/armstrong-numbers/.meta/solutions/armstrong_numbers.rb

@@ -0,0 +1,10 @@
+class ArmstrongNumbers
+  def self.is_valid?(number)


in Ruby I try not to use 'is_' prefixes. See: https://github.com/rubocop-hq/ruby-style-guide#bool-methods-prefix

What about: def self.armstrong?

(The name used in the tests will also need updating.)

I like self.valid? better than self.armstrong?, although it's not really about validity, is it? I can't think of a better name though. 🤔

In response to this comment I heavily considered suggesting is? because I thought it would be slightly amusing... but ArmstrongNumbers.is?(10) actually works in the opposite direction compared to is_a? so it would be unconventional.

If using include?, then ArmstrongNumbers.include?(10) does read naturally, but I think it would be surprising because it hints that you can use other Enumerable methods on ArmstrongNumbers (because include? is on Enumerable) but you cannot.

I really like include? 👍

It would be possible to implement this in a way that you could use Enumerable if you wanted to.

include? sounds nice, but I wonder how you'd like to implement it to use Enumerable? Even if we choose to do this using Enumerable I wonder (and if) students would use other methods from Enumerable? Perhaps we can settle on a better name / other implementation? I saw the Python example defines just one function: def armstrong_number?(number) ?

Including Enumerable is not really relevant to this PR but is something that could be interesting to explore as part of a solution.

exercises/armstrong-numbers/armstrong_numbers_test.rb

Insti · 2018-11-01T18:27:08Z

Looks good, I have noted a couple of things that need to be looked at. But it's overall a suitable implementation.
Not making a test generator is fine.

emcoding · 2018-11-01T21:51:18Z

Hi @Tuxified , @Insti!

What I like most is that it's an math exercise that doesn't ask to reimplement an existing Ruby method 👍

I feel using digits and sum would add value regarding the Ruby idiom, and make it a bit more about Ruby than about maths.
number.digits.sum { |digit| digit**exponent } == number

I don't think we should avoid Ruby 2.4 at all.
(Ruby 2.5 is adding fun: number.digits.sum { |digit| digit.pow(exponent) } == number , but let's not go there.)

It's an easy exercise regarding the math, but the Enumerable makes it more of a level unlocked by Isogram.
So:
Difficulty 3 seems fit, due to the Enumerable, topic: math, unlocked by Isogram

Note

I think I liked the singular class name better. Because it's a check, for a single number, not a collection of sorts. ??

Thoughts?

exercises/armstrong-numbers/armstrong_numbers_test.rb

Insti · 2018-11-02T05:09:09Z

I think I liked the singular class name better. Because it's a check, for a single number, not a collection of sorts. ??

I really agree with your logic here.

However the convention is that the class name always matches the exercise slug, so the s needs to be there.

See also: #854

emcoding · 2018-11-02T08:32:39Z

However the convention is that the class name always matches the exercise slug, so the s needs to be there.

~~It's a new exercise, so we can change the slug?~~ No, it's not. Sorry, didn't realize that.

emcoding · 2018-11-26T08:43:56Z

Hey @Tuxified What's the status?

and switch to Ruby 2.4 in the process

Tuxified · 2018-12-10T21:37:05Z

Apologies it took too long to respond, wasn't my intention to neglect your efforts. I've updated to include your comments, however I'm still unsure about the name of method and/or implementing it as an Enumerable.

Are you adamant on the Enumerable implementation or are you open for a different method name?

[edit]
I just saw my specs failed as Travies is running with Ruby 2.1.2, which doesn't include Integer.digits and Enumerable.sum .. so revert to old version with reduce and all?

emcoding · 2018-12-15T09:08:08Z

Hey Toni, thanks for the next round!

Looks to me that there's some confusion in the Enumerable discussion.
Let me try to recap and see if we're all talking about the same:

Candidates for name of the method:
- is_valid? : let's follow the styleguide and don't prefix predicate methods with is_...
- include? : does cover the meaning, but if I understand @petertseng 's comment correctly: as include? is also a method in Enumerable, using this name suggests that ArmstrongNumbers responds to other Enumerable methods as well. It's not about using Enumerable methods in the implementation of the method_to_be_named.
valid? is also a bit misleading, because it's mostly use for validations, and that's not exactly what we're doing; a non-armstrong-number is not per se invalid, it's just not an armstrong number.
I feel the slug = class name convention is making naming things even harder, but as we're stuck with it (for now), I got giggly and thought of: really? ?

Concluding:

Let's avoid names that start with is_, and names that have a meaning in and/or are part of Ruby.
I was thinking: maybe use a synonym for the english 'include', like contain? , or more like valid: confirm? ,affirm?, or more like checking: check?, proven?.

I'm okay of any of those, and I'd say let's just 'vote' for our preferences and let Tony decide.
(- Unless, of course, my recap of the discussion is off. -)

@Insti will be better at assessing the tests thing. All I know: there are exercises that use Ruby 2.4 and they work. I didn't look into that.

Insti · 2018-12-16T21:54:10Z

The tests and solution need to work with Ruby 2.1.2 so you'll need to either "downgrade" the code or write a "polyfill" to support the new syntax. Or create a new issue/PR to upgrade the minimum version of ruby we support.

Insti · 2018-12-16T21:57:03Z

@Tuxified
For the method name, instead of is_valid? use include?.
(Don't worry about the Enumerable stuff.)

guygastineau · 2019-08-06T04:10:32Z

Is this abandoned? I am willing to take over implementation of this exercise.
It has been a long time since I contributed here, but I am coming back to Ruby, so I figured helping here will help me ramp back up 😄

@Tuxified I don't want to step on your toes... If you aren't interested in finishing this I could build off of your work, so that you are still listed in the git history.

Tuxified · 2019-08-06T15:02:41Z

Hi Guy,

No problem, I lost track of this PR, so if you're willing to pick it up, plz do. No need to keep this PR BTW, I don't care that much about keeping git history :P

guygastineau · 2019-08-06T15:05:14Z

Cool. I'm on it.

guygastineau · 2019-08-08T01:08:45Z

@Tuxified I went ahead and rebased your work onto the current master, so I could keep your commits in #988 . I figured there was no reason to redo all the work from scratch, but I didn't want to take your work without credit 😈

Tuxified · 2019-08-08T09:40:14Z

Closing this, as #988 is a better choice :P

Tuxified added 3 commits October 31, 2018 23:47

Add Armstrong Numbers exercise

40af229

- not using Integer.digits as that is only available on Ruby 2.4

Update and rename armstrong_number.rb to armstrong_numbers.rb

9343e19

ArmstrongNumber -> ArmstrongNumbers

ad27c0b

complying with name convention

Insti requested a review from emcoding November 1, 2018 18:17

Insti reviewed Nov 1, 2018

View reviewed changes

exercises/armstrong-numbers/.meta/solutions/armstrong_numbers.rb Outdated Show resolved Hide resolved

Insti reviewed Nov 1, 2018

View reviewed changes

exercises/armstrong-numbers/armstrong_numbers_test.rb Show resolved Hide resolved

Insti reviewed Nov 1, 2018

View reviewed changes

exercises/armstrong-numbers/armstrong_numbers_test.rb Outdated Show resolved Hide resolved

emcoding reviewed Nov 1, 2018

View reviewed changes

exercises/armstrong-numbers/armstrong_numbers_test.rb Outdated Show resolved Hide resolved

Tuxified added 8 commits December 10, 2018 21:47

use underscore for large numbers

9fb5995

add skip to all but first test

fde0150

rename tests for better English

0c6963a

entry for config.json

19e3900

use chars instead of split('')

eb5a5dd

correct config.json syntax error

da27025

more idiomatic by using sum instead of reduce

96910d4

and switch to Ruby 2.4 in the process

correct slug to armstrong-numbers

9da8b9b

emcoding mentioned this pull request Dec 15, 2018

We need more exercises! #900

Closed

9 tasks

guygastineau mentioned this pull request Aug 8, 2019

armstrong-numbers: port #988

Merged

Tuxified closed this Aug 8, 2019

		@@ -0,0 +1,10 @@
		class ArmstrongNumbers
		def self.is_valid?(number)

Uh oh!

Add Armstrong Numbers exercise #893

Add Armstrong Numbers exercise #893

Uh oh!

Conversation

Tuxified commented Oct 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Insti commented Nov 1, 2018

Uh oh!

Uh oh!

Uh oh!

Insti Nov 1, 2018

Choose a reason for hiding this comment

Uh oh!

emcoding Nov 1, 2018

Choose a reason for hiding this comment

Uh oh!

petertseng Nov 1, 2018

Choose a reason for hiding this comment

Uh oh!

Insti Nov 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tuxified Dec 10, 2018

Choose a reason for hiding this comment

Uh oh!

Insti Dec 16, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Insti commented Nov 1, 2018

Uh oh!

emcoding commented Nov 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Insti commented Nov 2, 2018

Uh oh!

emcoding commented Nov 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emcoding commented Nov 26, 2018

Uh oh!

Tuxified commented Dec 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emcoding commented Dec 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Insti commented Dec 16, 2018

Uh oh!

Insti commented Dec 16, 2018

Uh oh!

guygastineau commented Aug 6, 2019

Uh oh!

Tuxified commented Aug 6, 2019

Uh oh!

guygastineau commented Aug 6, 2019

Uh oh!

guygastineau commented Aug 8, 2019

Uh oh!

Tuxified commented Aug 8, 2019

Uh oh!

Uh oh!

Tuxified commented Oct 31, 2018 •

edited

Loading

Insti Nov 2, 2018 •

edited

Loading

emcoding commented Nov 1, 2018 •

edited

Loading

emcoding commented Nov 2, 2018 •

edited

Loading

Tuxified commented Dec 10, 2018 •

edited

Loading

emcoding commented Dec 15, 2018 •

edited

Loading