CSV validation handling

[olivierobert]

Upon uploading the CSV file on the UI, the current validation could be improved to address the following:

• No user feedback when there are more than 100 keywords in the file. The below code simply hides the spinner without informing the end-user of the issue.

https://github.com/hudsonbay/google_scraper_live_view/blob/22ee211c8df4d802fe0879824e005b05ebb4aa95/lib/google_scraper_web/live/homepage_live/index.ex?_pjax=%23js-repo-pjax-container%3Afirst-of-type%2C%20div%5Bitemtype%3D%22http%3A%2F%2Fschema.org%2FSoftwareSourceCode%22%5D%20main%3Afirst-of-type%2C%20%5Bdata-pjax-container%5D%3Afirst-of-type#L59-L71

• Not hardcoding the keyword limit 🙈 Such rule should be placed in either a more prominent place or configurable place (or both).

https://github.com/hudsonbay/google_scraper_live_view/blob/22ee211c8df4d802fe0879824e005b05ebb4aa95/lib/google_scraper_web/live/homepage_live/index.ex?_pjax=%23js-repo-pjax-container%3Afirst-of-type%2C%20div%5Bitemtype%3D%22http%3A%2F%2Fschema.org%2FSoftwareSourceCode%22%5D%20main%3Afirst-of-type%2C%20%5Bdata-pjax-container%5D%3Afirst-of-type#L59

• Handling the validation in a module distinct from GoogleScraperWeb.HomepageLive.Index

[hudsonbay]

Not hardcoding the keyword limit

I think it's a good choice to extract it to a @keyword_limit variable

No user feedback when there are more than 100 keywords in the file. The below code simply hides the spinner without informing the end-user of the issue.

True. It needs fixing. Thanks

Handling the validation in a module distinct from GoogleScraperWeb.HomepageLive.Index

I'm afraid I don't understand that one

[olivierobert]

Let me try to explain the last point again. I also did not provide code examples for this 😅

What I meant is currently, the CS file content is validated in the LiveView with:

if Enum.count(contents) < 100 do
  #...
else
  IO.puts("LARGER THAN 100")
  #...
end

(The IO debugging should be reworked for sure 🙈 )

If we agree that the variable @keyword_limit should not be hardcode, one can see the advantage of having this limit and the related validations in another module:

defmodule CsvValidator do 
  @keyword_limit 100

  def validate(csv_content) do
     csv_content
    |> validate_limit()
    |> validate_duplicate()
    #...
  end

  depf validate_limit() do
    #...
  end
end

The benefits is to keep the LiveView focused on routing (handle_event) and controlling (handle_info) while keeping business logic away from the view 💡 Through this code challenge, application architecture is a crucial assessment criteria especially for senior software developers.

[hudsonbay]

(The IO debugging should be reworked for sure

Oh, it was 😄. It's on the PR

related validations in another module

keeping business logic away from the view

I'm not trying to agree on everything 😄, but you are very right and I have the same way of thinking as you and life as proven the importance of it