Hacker News Reader

Comment by morgante | Hacker News Reader

I can't believe this "refactoring" is actually presented as good—creating unnecessary abstractions for a simple function.

If anything, I would go in the opposite direction. There's no need to use parameterized variables at all. Just write out the message 3 times:

  private String generateGuessSentence(char candidate, int count) {
      if (count === 0) {
          return "There are no " + candidate + "s";
      } else if (count === 1) {
          return "There is 1 " + candidate;
      } else {
          return "There are " + count + " " + candidate + "s";
      }
  }

Maybe this isn't "clean" (I'd claim it is) but it's certainly the easiest to understand.

View on HN

Yes! I really like "99 Bottles of OOP" by Sandi Metz. I feel like it's a version of "Clean Code" that my brain actually aligns with.

In a chapter, talking about different ways you could write a class that generates verses for the song 99 bottles of beer, we end up at:

https://sandimetz.com/99bottles-sample-ruby#_shameless_green

    The Shameless Green solution is disturbing because, although the code is easy to understand, it makes no provision for change. In this particular case, the song is so unlikely to change that betting that the code is "good enough" should pay off. However, if you pretend that this problem is a proxy for a real, production application, the proper course of action is not so clear.

    When you DRY out duplication or create a method to name a bit of code, you add levels of indirection that make it more abstract. In theory these abstractions make code easier to understand and change, but in practice they often achieve the opposite. One of the biggest challenges of design is knowing when to stop, and deciding well requires making judgments about code.

This book is beyond parody. I send it to people I hate and want to sabotage, along with clean code.

So you’re actually sabotaging the people who have to maintain their code, not them.

...ok? Noted, you do not like the book I like.

Which means that in the future not only can you two ignore each other's book recommendations, you can also note each other's book disrecommendations to find stuff you'd actually enjoy! It's a win-win!

HAH! I kind of like the idea of a site where you can post recommendations for books, and subscribe to other people's recommendations, OR the inverse of other people's recommendations

Good idea. Other books I hate for you: Game programming patterns, Crafting Interpreters, the C# player's guide.

you mean like this place? https://www.goodreads.com/book/show/31183020-99-bottles-of-o...

I'd think your code is clean. The only change I might make is to change the if statements into a single switch with `case 0`, `case 1`, and `default`. But even without this, it's already much better than what Uncle Bob comes up with.

My preference would be not to refactor it at all. It works, you can figure out what it does, and the tests pass. Forget about it. Whoever wrote it, wrote it to the best of their ability to implement the requirements and style that existed at the time.

The requirements changed and it now has to handle a new case? Now you can refactor it. What refactoring to choose depends on the new case you have to support - it's going to look a lot different if you need to accept a new "language" parameter than if you need to special-case another number.

Agreed that I wouldn’t refactor this for no reason.

That being said, the first time I had to edit this (even just do change the base string) I’d make the change.

I'd remove the else if blocks since we're returning.

    private String generateGuessSentence(char candidate, int count) {
      if (count === 0) return "There are no " + candidate + "s";
      if (count === 1) return "There is 1 " + candidate;

      return "There are " + count + " " + candidate + "s";
    }

I feel like single-line conditionals are harder to read in bigger projects. I remove the brackets in single-statement conditionals and loops, but I still use another line.

Braces _always_.

It just needs one tired / inexperienced / somethingelse coder to "quickly add" an extra call to the topmost if and then you'll have interesting issues.

I'll take the "ugly" braces every day compared to having risky structures like that in the code at all.

Thorough testing should catch all those "interestig" issues.

But easier maintainability I also favor braces everywhere.

    return "There are no " + candidate + "s";

I always hated overloading + for both addition and concatenation. Hence D uses ~ for concatenation.

Why? It seems natural and I can't think of a case where it could be confusing off the top of my head (for strongly typed languages).

Algol was infamous for:

    "10" + 1

does that mean 11 or "101" ?

That shouldn't compile (or it should be an IDE error in interpreted languages). That doesn't seem to be the fault of operator overloading.

wat

https://www.destroyallsoftware.com/talks/wat

+ should be commutative.

I disagree - I think the intuition that mathematical addition being commutative means that the + operator must be commutative is not pragmatic. Whether + must be commutative is less relevant than whether + must be mathematical. I.e. I see no problem - theoretically, practically, or semantically - with using + to combine things in programming, mathematically or otherwise. As long as it's in a strong language that doesn't support nonsense. E.g. JavaScript might not deserve operator overloading (I imagine - I rarely use JS).

A few decades ago I refactored a different way:

   // this may be a mistake!  English language is unorthogonal in the extreme!
   const char * Add_es( int count ) { return 1 == count ?  "" : "es"; }
   const char * Add_s(  int count ) { return 1 == count ?  "" : "s" ; }

(some English words pluralize with "es" suffix, most with "s"). It has proven surprisingly useful:

   Msg( "%d file%s updated when you switched back", updates, Add_s( updates ) );

Only difference vs example is mine prints "0" vs "no".

I love this. The cognitive of understanding what this code does is super low, it solves the problem, presents no glaring performance issues, and isn’t trying to be cute.

This is so much easier to read for me personally. It immediately tells me what the test cases are. With the author's version it's not that obvious what all the possible combinations of verbIs, countStr and sIfPlural are. By eliminating redundancies the author introduces a new one, the count == 1 case is handled twice, once for verbIs and than again for sIfPlural.

This is much better. Love it when code follows the KISS principle without unnecessary abstractions or optimizations when its not needed.

I wanted to write a sane version of this function, and it looks exactly like this.

Sadly this only works for English. Of course, given the functional requirements, it's probably fine, but this kind of code exists IRL and it can be annoying to refactor to support more languages.

For example, many languages have grammatical gender and so may also change how the word looks like. Bonus points, when the word body changes. In this example it wouldn't change radically, but the word for a file in German is "die Datei" and the plural is "die Dateien" (although apparently "das File" can be used.)

Okay, this isn't that big of a deal, one might say. You just pack the word's singular and plural with the possible gender, and you're set. But no, because this isn't even the worst part! The simplest complicated case might be that you have a singular, dual, and plural. In some sense this makes sense, since "two" is pretty special in terms of amount of stuff.

But then you get stufi like the following, as documented in [0]:

> Three forms, special cases for numbers ending in 1 and 2, 3, 4, except those ending in 1[1-4] > > [...] > > Languages with this property include: > > Slavic family > > Russian, Ukrainian, Belarusian, Serbian, Croatian

Or the six form pluralisation found in Arabic.

So yeah, this sort of "complicated" pluralisation isn't just an issue if you're trying to support some comparatively very niche languages, but this is a thing one needs to think about when supporting many major world languages.

And of course, how you inflect the noun you're pluralising depends on the context of the sentence and the relevant grammar. For example, the form might change if you're at a place that expects a noun in its nominative versus accusative form. And of course, there's the order of words, and so muth more, that could be mentioned.

So yeah, natural language is difficult. Not quite Turing complete, but good localisation takes a lot of effort.

[0]: https://www.gnu.org/software/gettext/manual/html_node/Plural...

I agree!

or pattern match it:

   defmodule Guess do
      def guess_message(candidate, _count=0) do
        "There are no #{candidate}s"
      end

      def guess_message(candidate, _count=1) do
        "There is 1 #{candidate}"
      end

      def guess_message(candidate, count) do
        "There are #{count} #{candidate}s"
      end
    end

Reply on news.ycombinator.com