Hacker News new | ask | show | jobs
by itsmeknt 458 days ago
I tried scaling up the original into an intentionally convoluted nonsensical problem to see how a more complicated solution would look like for each approach. Do these look right? And which seems the most readable?

  # Functional approach
  
  var favoriteFoodsOfFurryPetsOfFamousAuthorsOfLongChineseBooksAboutHistory = books
    .filter(book => 
       book.pageCount > 100 and 
       book.language == "Chinese" and 
       book.subject == "History" and
       book.author.mentions > 10_000
    )
    .flatMap(book => book.author.pets)
    .filter(pet => pet.is_furry)
    .map(pet => pet.favoriteFood)
    .distinct()

  # Procedural approach
  
  var favoriteFoodsOfFurryPetsOfFamousAuthorsOfLongChineseBooksAboutHistory = set()
  for book in books:
    if len(book.pageCount > 100) and
       book.language == "Chinese" and
       book.subject == "History" and
       book.author.mentions > 10_000:
      for pet in book.author.pets:
        if pet.is_furry:
          favoriteFoodsOfFurryPetsOfFamousAuthorsOfLongChineseBooksAboutHistory.add(pet.favoriteFood)

  # Comprehension approach
  
  var favoriteFoodsOfFurryPetsOfFamousAuthorsOfLongChineseBooksAboutHistory = {
    pet.favoriteFood for pet in
      pets for pets in 
        [book.author.pets for book in 
          books if len(book.pageCount > 100) and
                   book.language == "Chinese" and
                   book.subject == "History" and
                   book.author.mentions > 10_000]
    if pet.is_furry
  }
FWIW, for more complex problems, I think the second one is the most readable.
2 comments

I'm more partial to the first one because it keeps a linear flow downwards, and a uniform structure. The second one kind of drifts off, and reshuffling parts of it is going to be … annoying. IME the dot style lends itself much better to restructuring.

Depending on language you might also have some `.flat_map` option available to drop the `.reduce`.

True! Good point on the restructuring, I haven't thought about it in that way.

I think I like the second approach because the loop behavior seems clearest, which helps me analyze the time complexity or when I want to skim the code quickly.

A syntax like something below would be perfect for me if it existed:

  var favoriteFoodsOfFurryPetsOfFamousAuthorsOfLongChineseBooksAboutHistory = books[i].author.pets[j].favoriteFood.distinct()
    where i = pagecount > 100,
              language == "Chinese",
              subject == "History",
              author.mentions > 10_000
    where j = is_furry == True
Hm, LINQ query syntax form is kinda going in that direction

  (from book in books
   where book.pagecount > 100 
        && book.language == "Chinese"
        && book.subject == "History"
        && book.author.mentions > 10_000
   from pet in book.author.pets
   where pet.is_furry == true
   select pet.favoriteFood)
  .Distinct()
But it also demonstrates the...erm, chronic "halfassedness" of LINQ's query syntax form with distinct() not available there and having to fall back to method syntax form anyway...
You would likely approach it in any style with some helper functions once whatever's in the parentheses or ifs starts feeling big. E.g. in the dot style you could

  fn bookFilter(book: Book) -> bool {
   return book.pageCount > 100 and 
     book.language == "Chinese" and 
     book.subject == "History" and
     book.author.mentions > 10_000
  }
  
  var favoriteFoodsOfFurryPetsOfFamousAuthorsOfLongChineseBooksAboutHistory = books
    .filter(bookFilter)
    .flatMap(book => book.author.pets)
    .filter(pet => pet.is_furry)
    .map(pet => pet.favoriteFood)
    .distinct()
Your FP example is needlessly complicated. No one who does FP regularly would write it like that.

  var favoriteFoodsOfFurryPetsOfFamousAuthorsOfLongChineseBooksAboutHistory = books
    .filter(book => 
       book.pageCount > 100 and 
       book.language == "Chinese" and 
       book.subject == "History" and
       book.author.mentions > 10_000
    )
    .flatMap(book => book.author.pets)
    .filter(pet => pet.is_furry)
    .map(pet => pet.favoriteFood)
    .distinct()
Or in Scala:

  val favoriteFoodsOfFurryPetsOfFamousAuthorsOfLongChineseBooksAboutHistory = (for {
    book <- books if
      book.pageCount > 100 &&
      book.language == "Chinese" &&
      book.subject == "History &&
      book.author.metnions > 10_000
    pet <- book.author.pets if
      pet.is_furry
  } yield pet.favoriteFood).distinct
Though, most Scala programmers would prefer higher-order functions over for-comprehensions for this.
I didn’t see the original, but the FP example here looks fairly idiomatic to me.

An alternative, which in FP-friendly languages would have almost identical performance, would be to make the shift in objects more explicit:

    var favoriteFoodsOfFurryPetsOfFamousAuthorsOfLongChineseBooksAboutHistory =
      books
        .filter(book => isLongChineseBookAboutHistory(book))
        .map(book => book.author)
        .filter(author => isFamous(author))
        .flatMap(author => author.pets)
        .filter(pet => pet.isFurry)
        .map(pet => pet.favouriteFood)
        .distinct()
I slightly prefer this style with such a long pipeline, because to me it’s now built from standard patterns with relatively simple and semantically meaningful descriptions of what fills their holes. Obviously there’s some subjective judgement involved with anything like this; for example, if the concept of an author being famous was a recurring one then I’d probably want it defined in one place like an `isFamous` function, but if this were the only place in the code that needed to make that decision, I might inline the comparison.
Thanks! I have updated my post to use your code. It is indeed much nicer. And yes, I don't write much FP.

I just improved the comprehension code as well using the same idea as your code, eliminating an entire list!