Factorial and test unit for factorial by hemahg · Pull Request #2 · srijitamukherjee/RoR-Learning-Path

hemahg · 2019-10-03T04:32:11Z

No description provided.

hallelujah · 2019-10-04T13:00:31Z

+        return result
+      else
+      power = -1*power
+        for i in (1..power) do


Or you could have done:

1.0/pow(num, -power)

But that is fine ;)

hallelujah · 2019-10-04T13:01:33Z

+  class Power
+    def self.pow(num,power)
+      result = 1.0
+      if(power >=0)


Parenthesis are not mandatory

hallelujah · 2019-10-04T13:02:06Z

+        for i in (1..power) do
+         result = result*num
+        end
+        return result


You can return result at the end of the method

hallelujah · 2019-10-04T13:02:47Z

+        for i in (1..power) do
+          result = result*(1/num)
+        end
+        return result.round(9)


That is acceptable for the purpose of the excercise

hallelujah · 2019-10-04T13:06:43Z

+       @n = @n - 1
+     end
+     result
+   end


Be wary of indentation, it makes the code more readable.
You probably want to setup your IDE with correctly.

That makes me think of a good read for start https://github.com/rubocop-hq/ruby-style-guide

hallelujah · 2019-10-04T13:07:10Z

+
+
+
+class Facttest < Test::Unit::TestCase


Please follow convention for file and directory namingnaming

hallelujah · 2019-10-04T13:08:26Z

@@ -0,0 +1,21 @@
+require "test/unit"
+require_relative "/home/hema/RoR-Learning-Path/Programming Foundations: Algorithms/Factorial-test/fact"


Do not require_relative

Rubyists generally run test with include in load path option -I. e.g: ruby -Ilib:test test/my_example_test.rb

hallelujah · 2019-10-04T13:09:00Z

+    assert_equal(6, Fact.new(3).fact)
+  end
+
+  def test_Zero


Follow convention see https://github.com/srijitamukherjee/RoR-Learning-Path/pull/2/files#r331490367

hallelujah · 2019-10-04T13:09:22Z

+    assert_equal(1, Fact.new(0).fact)
+  end
+
+  def test_negative


Make it more readable
See https://github.com/srijitamukherjee/RoR-Learning-Path/pull/2/files#r331490367

Martouta · 2019-10-14T12:23:41Z

+require 'first_project/power'
+
+class PowerTest < TestBase
+  test 'powerofnumber' do


since the name is just a string, i think that it is easier to read like this: 😄

Suggested change

test 'powerofnumber' do

test 'power of number' do

And then when you run the test, it will convert it internally to power_of_number which is how we name methods in Ruby (this naming convention is called snake case)

Martouta · 2019-10-14T12:47:27Z

+    def self.pow(num, power)
+      result = 1
+      if power >= 0
+        for i in (1..power) do


What I am gonna say in this comment, I don't know if it is appropiate or if it is too early (too difficult at this stage), so if you find it too hard right now, do not worry and just ignore this comment 😂

In Ruby we usually write this for i in (1..power) do like (1..power).each do |i| and what it does is: for the instance (1..power) of the class Range, call its instance method each and run the block after the do with the i as the parameter of the block.

Alternatively, as you do not mind about the index at all, you can also use the instance method times of the class Integer, which you can use when either you don't care about the value of the index, or when it can start by 0.

So it would be:

power.times { result *= num }

Considering that { ... } is the same as do ... end 😄 .

You can also use inject of an Enumerable (which is mostly any object that can be iterated, like a Range or an Array for example), and then you do not need the result variable and it is shorter 😄 Although it is too inneficient for this and I wouldn't recommend it, but since this is for the sake of learning, it would be:

def self.pow(num, power) if power >= 0 ([num] * power).inject(1) { |n, result| result * n } # Or even: ([num] * power).inject(:*) || 1 else 1.0 / pow(num, -power) end end

What ([num] * power).inject(1) { |n, result| result * n } does is, first make an array of power elements, all of them containing num, and then iterate all of them, with a variable called result starting by 1, and for each iteration of the array save in that variable the value of result * num, which is what returns at the end.

And there could be also a recursive version of this method, which in code is usually simpler but you also need to check to what it does with the stack of memory 😄 This version would be:

def self.pow(num, power) if power == 0 1 elsif power > 0 num * pow(num, power - 1) else # power < 0 1.0 / pow(num, -power) end end

And there are probably more efficient versions but for now this is enough 😄

Martouta · 2019-10-14T14:18:52Z

@@ -0,0 +1,15 @@
+module Hema
+  class Power


We spoke in private before but I rather put it here as well.
We don't usually create a class if we don't expect it to be ever instantiate it and for that we usually just make a module instead. Like this:

module Hema module Power def pow(...) ... end end end

And from outside called like: Hema::Power.pow(...)

hallelujah

Implementation is good.

Now all comments are about Styling and using Ruby Core Library methods

hallelujah · 2019-10-15T06:41:47Z

+    end
+
+    def fact
+      if @n == 0


We try to avoid many nested if for learning you have https://rubystyle.guide

You can also use rubocop to tell you what is bad styling in your code https://github.com/rubocop-hq/rubocop

hallelujah · 2019-10-15T06:44:25Z

+        raise 'negatives are not acceptable'
+      else
+        result = 1
+        while @n > 0


As stateed by @Martouta, when you have Enumerable, try to read the docs and use some of the methods of this module.

Try to understand this simpler version is:

(1..n).inject(&:*)

hallelujah · 2019-10-15T06:45:02Z

+
+  def test_negative
+    Factorial_test::Fact.new(-5).fact
+    assert_raise do


This does not work as you do not execute nothing inside the block.

hallelujah · 2019-10-15T06:45:39Z

+class PowerTest < TestBase
+  test 'powerofnumber' do
+		assert_equal 4, Hema::Power.pow(2, 2)
+		assert_equal 1, Hema::Power.pow(2.0, 0)


There are formatting/styling issues, see https://rubystyle.guide

hema added 10 commits September 26, 2019 18:38

Added file

2c75cc2

Added file to ruby basics

c90f244

Pattern program

264c74e

Added test-unit

055d8c1

Removed Facttest.rb file

659e4bc

Factorail and factorial test_unit

fe46453

Updating branch

5277bc9

Creating floder structure of the project

2088001

Creating unit test for power.rb

d5e66d1

Implementing logic to find the power of number

c25f5e3

hallelujah reviewed Oct 4, 2019

View reviewed changes

Incorporated review comments

dab36a4

Martouta reviewed Oct 14, 2019

View reviewed changes

hallelujah reviewed Oct 15, 2019

View reviewed changes

Updated factorial test

21af9a1

		@@ -0,0 +1,21 @@
		require "test/unit"
		require_relative "/home/hema/RoR-Learning-Path/Programming Foundations: Algorithms/Factorial-test/fact"

Conversation

hemahg commented Oct 3, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hallelujah Oct 4, 2019 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hallelujah Oct 4, 2019 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Martouta Oct 14, 2019 • 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!

Choose a reason for hiding this comment

Uh oh!

Martouta Oct 14, 2019 • 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!

hallelujah left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hallelujah Oct 4, 2019 •

edited

Loading

hallelujah Oct 4, 2019 •

edited

Loading

Martouta Oct 14, 2019 •

edited

Loading

Martouta Oct 14, 2019 •

edited

Loading