6
votes
Accepted
Rails Helper to display rating in stars
You can make use of the Array.new, passing in the maximum number of stars you want to show, and defaulting all the stars to empty. Then, you can ...
- 251
6
votes
Accepted
How can I make this CSV Importer code better of any code smell?
I like that you already extracted a WebCsvImporter Plain Old Ruby Object which makes the code already a lot easier to understand and refactor.
Here is a refactored ...
4
votes
Accepted
Presenter pattern implementation
At this point we can already see that CartPresenter (as others) needs to embed Model's data into HTML. As it violates the Single Responsibility Principle, do I need something like Decorators to take ...
4
votes
Accepted
Ruby SFTP client
... under the terms of the GNU General Public License ...
IANAL, TINLA.
As copyright holder you chose
to license the code in your posting under the terms of
CC-BY-SA,
which is incompatible with GPLv3 ...
- 43.3k
3
votes
Accepted
URL Routing Application
You could do the following. (I assume you already have a resourceful route for your documents right?)
...
- 146
3
votes
Find associated object if exists, otherwise generate it
Expanding on @jpemberthy: Shouldn't we duck type?
def pick_for_game(game)
game = game.id if game.respond_to? :id
picks.find_or_initialize_by_game_id(game)
end
...
3
votes
Accepted
A method that calculates win streaks
You can use Array#min and Array#max to calculate the win/lose streak;
You don't need to use self on getters;
And you can use ...
- 215
3
votes
Validate if the user is associated with the organization then it must have any valid timezone
In case if you wanna improve only the validate_time_zone method:
Performance wise I don't see anything sinister in your implementation.
I assume the ...
- 143
3
votes
Accepted
Update join table using list of checkboxes in Rails
Actually, Active Record can work with joined collection (including has_many :through) from the box.
So, you just need to pass new collection instead of old, and AR will delete excessive and add new ...
- 301
3
votes
Accepted
Dynamically combine ActiveRecord Relation results
Turns out the last code snippet in my question was close to what I ended up needing:
...
- 61
3
votes
Accepted
Rails: create a relationship between comment and user
Don't think your question belongs to this code review section. You would have gotten help much faster if had posted in stackoverflow instead. Anyway there are couple points you need to check.
The ...
- 166
3
votes
Accepted
Migration in a bioinformatics application that changes a column name and explains the rationale
Does this code follow common best practices in Ruby regarding block vs line comments?
No, it doesn't.
And with the fear of starting a flamewar here, it doesn't matter either as long as you understand ...
- 521
3
votes
Ruby CLI TicTacToe
Welcome to Code Review, you have almost the same code block repeated 9 times:
...
- 28.2k
3
votes
Ruby on Rails _form implicit partial test with RSpec
The real question here is not if this is good implementation but what value these tests provide against regression.
For example when you're testing:
...
- 606
3
votes
Accepted
Rails session after migrate from PHP (improved)
Regarding the PHP part, you can make it a bit simpler
$result = password_verify($password, $hased_password);
exit((int)$result);
- 9,143
2
votes
Duplication of enum across two models
In Rails 6, you can put the enum in a module in concerns and include it in your models.
model/concerns/statusable.rb
...
2
votes
Avoiding Duplicates in a Ruby on Rails Factory Girl Factory with Fake
This can be accomplished now with:
name { Faker::Company.unique.name }
- 121
2
votes
Querying multiple related records in Rails
I would try to use as much as possible what is provided by ActiveRecord. I was thinking to something like:
...
- 156
2
votes
Accepted
Rails model for publishable posts
You could just do:
def published?
!hidden? && published_at.present? && published_at < DateTime.now
end
However I would probably define ...
- 3,547
2
votes
Shopping cart in Rails for karaoke items
Generally looks great.
Since @cart is basically a list of Karaoke ids, you could update the ...
- 141
2
votes
Ensure multiple steps work/saving multiple models via a service
A couple of issues I see here:
What happens if an Author is saved, and then Tomato creation fails? Should the author record ...
- 121
2
votes
Accepted
RSpec test for handling known IP addresses
I'll use this known_ips somewhere else.
I'm guessing that "somewhere" will still be in a test file. If so you can put this in support file (which is a common patern in RSpec).
...
- 215
2
votes
Accepted
Pyramid of doom Grape endpoints
You don't have to define the whole flow per route, this still leads you to the pyramid of hell.
...
2
votes
Rspec test for Rails method to return alphabetically sorted list of tags
Your test code is merely a copy of your model code, so you're not really testing anything.
Rather, your unit test should rely on fixtures, and the expected answer should be hard-coded in the test.
- 146k
2
votes
Accepted
Ruby on Rails - Creating many models associated with another one
You should be able to do this with accepts_nested_attributes_for.
see https://api.rubyonrails.org/classes/ActiveRecord/NestedAttributes/ClassMethods.html#method-i-...
- 3,547
2
votes
Accepted
Re-creating user discounts nightly
The first thing that I noticed that could be improved is to use find_each instead of all.
...
- 191
2
votes
Easily interacting with two external apis
def bigcommerce_api_v3_get_customers_by_shop(shop, options = {})
Since you are only extracting a couple values from options ...
- 260
2
votes
Removing duplicate hashes in an array while keeping count of the times the hash was present
Does it have to be an Array? I.e. is the order important?
If not, then there is actually a data structure that does exactly what you want: the multiset. A multiset ...
- 2,784
2
votes
RSpec shared example for CRUD Controllers that might be nested or not
A couple notes.
1) You don't have to pass :each to before calls, it's the default:
...
- 141
Only top scored, non community-wiki answers of a minimum length are eligible