Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix NoMethodError on list_neighborhoods with subdistrict #43

Merged
merged 1 commit into from
Jun 29, 2020
Merged

Fix NoMethodError on list_neighborhoods with subdistrict #43

merged 1 commit into from
Jun 29, 2020

Conversation

sarslanoglu
Copy link
Owner

@sarslanoglu sarslanoglu commented Jun 13, 2020 •
edited
Loading

Related github issue for this PR

Resolves #38

Checklist

  • I have performed a self-review of my own code
  • I added comments, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation README.md
  • I added tests that prove my fix is effective or that my feature works
  • bundle exec rake passes locally
  • rubocop gives no error locally
  • My PR title includes "WIP" or is draft PR if work is in progress

Description

TurkishCities.list_neighborhoods('Eskişehir', 'Odunpazarı', 'Büyükkkkkdere') method was given NoMethodError (undefined method `[]' for nil:NilClass)

It needs to be clear that user has send wrong parameters to method

With this change it returns "Couldn't find subdistrict with 'Büyükkkkkdere' of 'Odunpazarı'/'Eskişehir'"

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Which tests are written for this PR? And what are the expected results for these tests?

One test added to

describe '#list_neighborhoods' do
  context 'when input is not supported' do
    it 'gives city_not_found_error' do

@sarslanoglu sarslanoglu added the bug Something isn't working label Jun 13, 2020
@sarslanoglu sarslanoglu self-assigned this Jun 13, 2020
enderahmetyurt
enderahmetyurt reviewed Jun 13, 2020
View reviewed changes
lib/turkish_cities/district.rb
sort_alphabetically(neighborhoods)
neighborhoods = create_neighborhoods(district_list, city_name,
district_name, subdistrict_name)
neighborhoods.is_a?(Array) ? sort_alphabetically(neighborhoods) : neighborhoods
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

belki bir bosluk koyulabilir daha rahat okumak icin. yani 32. satira alinabilir.

lib/turkish_cities/district.rb
else
district_not_found_error(district_name, city_name)
end
end

private

def create_neighborhoods(district_list, city_name, district_name, subdistrict_name)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bu method kafami yakti :) nasil duzeltebiliriz? cok fazla parametre almis. belki oradan baslamak lazim.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@enderahmetyurt Kusura bakma bir kaç haftadır bakamadım repo'ya anca vakit bulabildim. Benim de aklıma bir şey gelmedi kolaylaştırmak için. Unutulmasın diye issue açtım #45

@sarslanoglu sarslanoglu mentioned this pull request Jun 29, 2020
Closed
@sarslanoglu sarslanoglu merged commit 91e58f6 into master Jun 29, 2020
@sarslanoglu sarslanoglu deleted the feature/issue_38 branch June 29, 2020 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enderahmetyurt enderahmetyurt enderahmetyurt left review comments

Assignees

@sarslanoglu sarslanoglu

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] - NoMethodError on list_neighborhoods
2 participants
@sarslanoglu @enderahmetyurt