web api #26
web api #26
Conversation
Christopher
integration of everything
Integration
Fixed the generated csv
Fixed typos
| "url": "https://swapi.co/api/people/10/" | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
I think It's a good idea to store the data of api in files to use them as backup. Good Job!
There was a problem hiding this comment.
you're right. we let it slip. thanks for making us notice it :)
| - Mejorar interfaz | ||
| - Switch using api and local storage | ||
| - Validaciones rspec | ||
| - Validar que son varias páginas |
There was a problem hiding this comment.
I like your list of pending tasks. 👍
There was a problem hiding this comment.
Visual Studio Code has many plugins that identify the TODO tag. This can be very helpful to coordinate group work.
| " 1. Most expensive vehicles", | ||
| " 2. Fastest vehicles", | ||
| " 3. Tallest people", | ||
| " 4. Heaviest people", |
There was a problem hiding this comment.
Can you tell me how this information about overweight people could be significant to us?
There was a problem hiding this comment.
It was just a fact, we didn't really had any other in mind. You're right. We'll probably change it if we release another version of this :) thanks for noticing it.
| end | ||
|
|
||
| def parse_values_to_i(arr_bidimensional) | ||
| arr_bidimensional.map! do |i| |
There was a problem hiding this comment.
Why do you use map! instead of map ?
There was a problem hiding this comment.
oh, you're right! we just realized it wasn't necessary 😅
|
Missing specs |
|
|
||
| def get_results(json_complete) | ||
| json_complete[RELEVANT_DATA] | ||
| end |
There was a problem hiding this comment.
Organize your code is very good, but I think it is unnecesary create a method for only 1 line of code, it would be more readable if the line 47 use directly in the places where you use the method.
| def fetch_data(query) | ||
| @http = HTTP.get(BASE_URL + query + "/") | ||
| JSON.parse(@http) | ||
| end |
There was a problem hiding this comment.
Be careful! Revise good the api that you use, because it sometimes come divide for pages, in this case if you revise the link https://swapi.co/api/people/, in the key "Next" appear a value that show you the rest of information is in page 2.
| "url": "https://swapi.co/api/vehicles/24/" | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
Good Job! looks very organized :)
| } | ||
| top | ||
| end | ||
|
|
There was a problem hiding this comment.
The method "times" seems to me very interesting to use, but it would be more simple and short if your information store in a array so if you want get only a specify amount you would just need to use the index final to return the information.
| @printable_data = [] | ||
| end | ||
|
|
||
| def give_me_the_top |
There was a problem hiding this comment.
How can this method be renamed to be more clear? What does it build?
| json_complete[RELEVANT_DATA] | ||
| end | ||
|
|
||
| def get_main_attribute(result, main_attribute) |
There was a problem hiding this comment.
I prefer methods that aren't named with the get_ prefix, it looks cleaner 😄
| results = get_results(json_answer) | ||
| main_attributes = get_main_attribute(results, @name_main_attr) | ||
| top_ordered = get_top_of_results(main_attributes, results) | ||
| final_top = build_final_top(@relevant_keys, top_ordered) |
There was a problem hiding this comment.
@printable_data = ...
| @@ -0,0 +1,21 @@ | |||
| ### Use instructions | |||
|
|
|||
| ```bash | |||
There was a problem hiding this comment.
Should be shell instead of bash. Bash is one of the available system shells and we are using zsh.
| - Mejorar interfaz | ||
| - Switch using api and local storage | ||
| - Validaciones rspec | ||
| - Validar que son varias páginas |
There was a problem hiding this comment.
Program accomplishes what's required, but I would appreciate if the a small description of the parts is available.
| @tops = tops | ||
| end | ||
|
|
||
| def run |
There was a problem hiding this comment.
Should the system class include the main procedure to execute the program?
| require "csv" | ||
|
|
||
| class System | ||
| TEXT_INSTRUCTIONS = [ |
There was a problem hiding this comment.
To reuse the class, this array can be an outside input.
| def run | ||
| print_instructions | ||
| chosen_number = get_chosen_number | ||
| system "clear" |
There was a problem hiding this comment.
Don't forget that this command is *nix only. Won't work in Windows machines.
| end | ||
|
|
||
| def get_chosen_number | ||
| numero_top = gets.to_i |
There was a problem hiding this comment.
Won't work if input is different from the menu's range.
Team Valeria, Paul y Christopher