Hacker News Reader

Comment by syklemil | Hacker News Reader

fwiw, once Python's introduced there's the third option on the table, comprehensions, which will also be suggested by linters to avoid lambdas:

    authors_of_long_books: set[Author] = {book.author for book in books if book.page_count > 1000}

These are somewhat contentious as they can get overly complex, but for this case it should be small & clear enough for any Python programmer.

View on HN

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.

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

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()

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...

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.

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!

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.

Without syntax highlighting, "book.author for book in books if book.page_count > 1000" requires a lot more effort to parse because white space like newlines is not being used to separate things out.

Set comprehensions are normal in mathematics and, barring very long complex ones, I find them the easiest to parse because they are so natural.

They're just a tad more verbose in Python than mathematics because it uses words like 'for' and 'in' instead of symbols.

    authors_of_long_books: set[Author] = {
        book.author 
        for book in books 
        if book.page_count > 1000
    }

You've had some answers already, but I also think this is a good argument for syntax highlighting. With tools like tree-sitter it's pretty easy these days to get high quality syntax highlighting, which allows us humans to receive more information in parallel. A lot of the information we pick up in our daily lives is carried through color, and being colorblind is generally seen as a disability (albeit often a mild one which can be undetected for decades).

Syntax highlighting in print is more limited because of technological and economic constraints, which might leave just bold, italics and underlines on the table, while dropping color. On screens and especially in our editors where we see the most code, a lack of color is often a self-imposed limitation.

That's not the point though. If you need the syntax highlighting to quickly make out the structure, perhaps the visual layout is not as good as it could be.

I consider syntax highlighting to be a part of the _visual_ structure. Visibility is more than just whitespace and placement!

Set comprehension are more idiomatic here (explicit syntax) though filter/map are not that bad too:

    {*map(_.author, filter(_.page_count > 1000, books))}

It uses lambdas package.

Reply on news.ycombinator.com