Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

There is definitely a spectrum of "knowledge" at play when it comes to these considerations. The most obvious DRY violations are those kinds of things that you go "oh I need to test for this case" because that is usually an indication of some knowledge you need to know when interacting with a piece of code. EG, if you ever use -1 as a sentinel value then the knowledge of "what -1" means should be consolidated together, otherwise all clients will have to know that -1 is a sentinel, what it means and at best you'll have duplicate code, at worst those interpretations won't align and you might have a subtle bug where that -1 is doing something somewhere (ie it is supposed to mean "No information provided" but somewhere something is keeping an arithmetic mean of this field and those -1s are now screwing up your metrics and you don't really notice).

When we think about the knowledge of "how to do something" that's where things can get confusing. 9/10 times I'd say that right move is to look for common assumptions or facts. IE it isn't just "doing something" that is important, but the assumptions made in the process of doing it:

As an example, consider finding the average word length in some piece of text. We might start writing that feature like:

  def count_words(text: str) -> int:
      return len(text.split(' '))

  def average_word_length(text: str) -> int:
      num_words = count_words(text)
      word_lengths = []
      for word in text.split(' '):
          word_lengths.append(len(word))
      return sum(word_lengths) / num_words


then the piece of knowledge they share is "what a word is" and the DRY refactoring would pull out that piece of knowledge into its own function

  def words(text: str) -> List[str]:
      return text.split(' ')

that might be code you write when starting to write a feature and that's the kind of "ding ding ding there's common knowledge here" that should guide refactoring. The system has a concept of a "word" that we've introduced and its important that knowledge about "what a word is" in one place. For DRY things it frequently doesn't make any sense for there to be multiple statements of "what a word is" where the system wants to use the same concept.

Kind of orthogonal to this is abstraction where the focus is on "usefulness" and that is where 100% you can abstract incorrectly, prematurely, get screwed over by requirement changes, write a library that hides everything and makes people angry. The example you provide seems more like an error in abstraction where things that should be close together are too far apart in the system (ie, some "fact" is hidden away and another part of the system wants to know it), but the consolidation and DRYing of those facts, I'd argue, is a lot easier once we've figured out how to identify them



Yeah, I like this approach, because the "what is a word" knowledge is a nice piece of common functionality that doesn't make sense to repeat. It's unlikely to change for just one of those two functions.

In my example, it's less a "core piece of knowledge" that people are trying to DRY, and more just a "common sequence." Someone sees a bunch of different places where we have a sequence of calls like A, B, C, D.. and says "oh this is a shared method I can extract" even if there's plenty of ways that in the future you might want to do A, B, C, E without D. And so then you pass in a bool, than another one, and you have a centralized mess...


I think the distinction is that if those two pieces of code had a different idea of what a word is then that would constitute a bug, then you definitely need to replicate the 'how to find words' logic. But if it doesn't really matter if two different pieces of code are using the same exact way to do something, then that's likely 'coincidental' replication. If you need to do word splitting, and someone else has written a word splitter, by all means copy paste their code to get you started, but definitely don't assume the best plan is to pull their code in as a dependency.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: