search

EdCrux / Enumerables-Ruby-methods

In this repository I want to show the main methods found in the module Enumerables in Ruby
1 stars 0 forks source link

Recreating Ruby's Enumerable #3

Closed EdCrux closed 4 years ago

EdCrux commented 4 years ago

Thanks in advance for the review. 🚀 This is the development summarize path of recreating Ruby's Enumerable. my_each, my_each_with_index, my_select, my_all?, my_any? , my_none? my_counter? my_map? and my_inject?

Anna-Myzukina commented 4 years ago

Pull Request under the review by Anna Muzykina (TSE)

Anna-Myzukina commented 4 years ago

Hi @EdCrux !

Status: [changes required]

Next methods still working not correct:

Make changes and submit another code review request.

If you have any questions, let me know on slack (@Anna-Muzykina) and I will be glad to clarify with you. Happy coding

shshamim63 commented 4 years ago

Status :spades: [Required Changes] :spades:

Hi @EdCrux :wave: , Awesome job :clap. Code looks neat and clean and you have covered all the changes asked by the previous reviewer. But I have found three cases where some of the methods malfunctions. The good news is by the pace you are covering the issue I am sure you will nail those too. So lets get to those issues.

:space_invader: [Mandatory Changes] :space_invader:

  1. my_any? identical and returns the same thing as ruby's any?. (Screenshot from Odin) https://github.com/EdCrux/Enumerables-Ruby-methods/blob/cac652b5daf626d790c635fce5d6ecddf8593d86/enumerables.rb#L72-L86 ...1. enumerables my_any? when no block or argument is given returns true if at least one of the collection is not false or nil. 
    puts [1, nil, false].my_any?   # should return true.
puts [nil, nil, false].my_any?   # should return false. Which I am getting on your one.
  2. my_inject identical and returns the same thing as ruby's inject. (Screenshot from Odin) https://github.com/EdCrux/Enumerables-Ruby-methods/blob/cac652b5daf626d790c635fce5d6ecddf8593d86/enumerables.rb#L128-L142 ...1. my_inject, when a symbol is specified combines each element of the collection by applying the symbol as a named method. But for some cases I am not getting the desired ans that is required. For an example lets execute the following code, after the end of your enumerable module 
    ARRAY_SIZE = 5
LOWEST_VALUE = 0
HIGHEST_VALUE = 9
array = Array.new(ARRAY_SIZE) { rand(LOWEST_VALUE...HIGHEST_VALUE) }
puts array.my_inject(:+)
puts array.inject(:+)

    should get same result printed twice. But not rendering it.

    :clubs: [Optional Changes] :clubs:

  3. multiply_els that uses my_inject as described on Odin (Screenshot from Odin) There must be a method called multiply_els which uses my_inject. The main purpose of that method is to check your enumerable methods are accessible from outside and you know how to use them properly. But I can not see that on this project. And I am sorry that the previous reviewer did not mention about that one too. So marking this one as optional.

    Please fix the above mentioned issue and send another review request.

    :herb: Happy Coding :herb:

    Shakhawat Hossain

    Linkedin