7 ms·

Programming like a Pirate (Alt+Shift+M)

59 points by magnars 14y ago

brnstz 14y ago
I agree that unit tests for internal (non public) functionality are not particularly useful, and can hamper your willingness to change an internal implementation for the better. I also agree it's painful to trace through dozens of classes/methods to determine what the actual implementation is. However, I don't agree the culprit is small methods. Small, interdependent methods in a single class (or package) are great and can be easy to follow if they are consistent. Usually the culprit is overuse of inheritance and design patterns. However, one case where I find myself being less "fancy" is functions that run/generate SQL statements. Although there are many cases where you can re-use certain portions of SQL statements and build up a query through smaller functions, this ends up being a nightmare to debug. I find it is better to be as plain and explicit as possible.
nnq 14y ago
on CleanCoder: I know $1 is just $1... but putting a sample video that I can just click to see would help a lot
BruceIV 14y ago
Hear, hear - I've been marking undergrad coding assignments all term, and the ones where they make a new method for absolutely every block of their code are nearly as unreadable as the ones that don't know how to indent or pick good variable names.
- d0m 14y ago
  Mind to show an example? Does the new method have a meaningful name? Are they smartly structured in a hierarchical order? I find it hard to believe that too much methods are harder to read than bad variables or indentation.
  - BruceIV 14y ago
    Keep in mind that these are undergrad projects, so a few hundred LOC tops. But you'll see something like: bool noMoreElves() { return elfCount == 0; } void fireAllReindeer() { for (int i = 0; i < reindeerCount; ++i) { delete reindeerArray[i]; } delete reindeerArray; } void checkCloseWorkshop() { if ( noMoreElves() ) { fireAllReindeer(); } } the above is certainly readable, but takes forever to parse -- the below is, I would argue, just as clear, and much quicker to read (the point the OP made) void checkCloseWorkshop() { if ( elfCount == 0 ) { // fire all reindeer for (int i = 0; i < reindeerCount; ++i) { delete reindeerArray[i]; } delete reindeerArray; } } the comment above the loop is possibly superfluous, but helpful
    - skywalk 14y ago
      That goes against many recommendations I've read about refactoring and writing cleaner code: "Good code invariably has small methods and small objects. I get lots of resistance to this idea, especially from experienced developers, but no one thing I do to systems provides as much help as breaking it into more pieces." – Kent Beck
      - dalke 14y ago
        The little research I've seen has not backed up that opinion. While that view is shared by many - especially those who come from a Smalltalk heritage - the studies I've read about do not show a clear association between small pieces and more readable/maintainable code, at least not for code blocks under 100 lines or so. I dislike reading Smalltalk influenced code with many small pieces. They tend to use instance state as a way to pass parameters, and that makes me uncomfortable - I have a leaning towards functional programming. I also distrust function names. A function like "noMoreElves" might actually be "no more elves!" and implemented as "elfCount = 100". I exaggerate a little, but an inline "elfCount == 0" is much easier to verify than having to jump elsewhere to see the code.
      - ramblerman 14y ago
        http://stackoverflow.com/questions/153234/how-deep-are-your-unit-tests/153565#153565 http://stackoverflow.com/questions/153234/how-deep-are-your-... Kent beck also gave this amazing (and suprising) answer on SO about "how far to go with your tests?" He is in my eyes, a true pragmatist and gives some sound advice. Uncle bob on the other hand seems to have drunk too much of his own koolaid. I saw him give a talk 2 years ago at a java conference which made him seem more like a Steven Balmeresque salestype, not an active developer.
dexen 14y ago
> Did you ever learn how to read code at university? Were you shown loads of different code, in different languages and different styles, and then asked to explain what the code did? I don’t think code comprehension is a normal part of the curriculum for computing science classes. We’re taught how to write, not how to read. Maybe that’s a mistake. Dear lazyweb, I want to bridge that gap in my education. Where do I go to read loads of code in different styles? With the emphasis on plurality of styles.
- indiecore 14y ago
  Github?
  - robertfw 14y ago
    I would recommend the above, along with chapters from http://www.aosabook.org/en/pypy.html http://www.aosabook.org/en/pypy.html as a guide
- dalke 14y ago
  I'll augment indiecore's answer. What kind of code are you interested in? Suppose you are, like me, interested in cheminformatics software. Find a few different packages (Open Babel, RDKit, Indigo, and others), download them, and compare how they implement a given task, like SMILES parsing or fingerprint generation. If you're interested in text compression, then download a few compression toolkits and see how they work. Also read a few articles on the topic. Regular expression engines? Dozens are available. Interested in networking? Take a look at ZeroMQ and other networking packages. Databases? That's a big topic, but you don't need to understand the entire package. Pick some aspect which interests you and see if you can figure out how SQLite, PostgreSQL, and MySQL handle it. Here's an easy one to start off with - how do Lua, Python, and C++ implement hash table-like data structures?
- aidenn0 14y ago
  Reading code without a purpose can cause your brain to tune out. You think you're learning but you aren't. I would rather suggest you find an open-source project and fix a bug in it. You will have to read lots of code in order to do so, and, so long as there is a repeatable test-case, fixing the bug shouldn't be too hard.
aidenn0 14y ago
I'm not sure I agree with this. If I'm looking at the implementation of a function, it's because it's done something wrong. For this function, I would fire up the debugger and say: "Does isTestPage() return what I expect? Yup, so the problem is going to be in surroundPageWithSetupsAndTeardowns()." A well named function becomes a black-box; you can tell if it's working just based on its inputs and outputs. Why open black boxes that are working?
- thibauts 14y ago
  > Why open black boxes that are working? Maybe to modify them ?
  - el_cuadrado 14y ago
    Well, modifying a distinct method is much easier than modifying something in a middle of 20-page scroll of code. There are simply fewer thing that can go wrong.
  - klibertp 14y ago
    Or even to understand how they are working and to check if they are really working correctly? I really don't like this attitude of ignoring inner workings of things just because they seem to work. Sometimes we really are working under constraints that make digging through the code impractical, but in my experience these constraints tend to be in the code itself - namely, someone wrote it as a 'black box' with tons of kludges and `if False:` style comments. I don't want to exaggerate and say that every programmer should know his every tool down to it's last bit, but taking black boxes apart is what makes one a hacker, and what allows one to improve one's skills.
    - aidenn0 14y ago
      Fair enough. Curiosity is a very reasonable excuse for opening a working black box, and is definitely what makes one a hacker. On the other hand, most systems these days are so complex that you have to have some black boxes. I know more about how my linux kernel works than the vast majority of people I work with, and yet I would classify the majority of it as a black box. I have a general idea of various trade-offs in efficiency and fragmentation resistance in malloc implementations, and if I have to I can dig into one. On the other hand, jemalloc works great and I just don't have the time to take it apart, so I just use it. Ultimately to get things done you need to trust that functions do what they say they do until you get evidence to the contrary. Taking functions apart can make you better at doing your job, but rarely fixes the problem you have Right Now.
guard-of-terra 14y ago
The problem with if (page.hasAttribute(“Test”)) {… is when you then decide to change boolean attribute Test to an enum attribute Environment with values Development, Test and Production. Because now you have to hunt down all the page.hasAttribute(“Test”) in your code and replace them with something different. And if your code is creepy enough you will miss one or two and something will break. So you have to look very carefully whether you repeat the same checks. If you repeat a check in different places and it has some meaning, you should abstract it away.
- engtech 14y ago
  The bigger problem I had with that snippet is the exposed string. I haven't seen the original video this is from, but I can't wonder but removing this hardcoded string was the bigger reason. Encoded strings like this are a maintenance problem lying in wait. How do you grep/search for it? These are all equivalent: if (page.hasAttribute("Test")) {… if (page.hasAttribute('Test')) {… string test = "Test"; if (page.hasAttribute(test)) {… string cheese = "Test"; if (page.hasAttribute(cheese)) {…
- hapless 14y ago
  The core problem is that it moves part of your OO interface into data. With "isTestPage()" you have a method with well-specified outputs. With "page.hasAttribute('Test')," a load of things become part of the permanent interface of the class: hasAttribute(), the datatype of 'Test,' the content of the field in the constructed argument etc...
- anonymous 14y ago
  That's simple - change the method so all those invocations are invalid now. If you're using a compiled language, the compiler will tell you all the lines that need changing. If you're using a dynamic language, you should have test coverage for all those methods and your tests will blow up with "no such method". Or maybe, you could special-case the 'hasAttribute(name)' method so it logs a debug warning and checks the enum attribute. Really, there are enough ways to deal with that and you're far from the first person to have had that problem.
beatgammit 14y ago
Is Alt+Shift+M significant to the article? I don't normally code in IDEs.
- tltjr 14y ago
  Alt+Shift+M extracts text to a method in Eclipse.
teamonkey 14y ago
Programming like a pirate? I was expecting this to be about R.
killnine 14y ago
When you look at the table of contents and think chapter 5 looks interesting, you don’t want chapter 5 to simply be another table of contents - which in turn links to a third table of contents etc yes!
pnathan 14y ago
This is part of the art of our work as software craftspeople; choosing when to refactor and when not to. There's no great joy in reading 400-line functions, and no great joy in hunting down 10 levels of the call stack to find out what is going on.
derefr 14y ago
> Think about this, in your experience - why do go have a look at the implementation of a method? Either you need to change it somehow, or it isn’t behaving as you thought it would. When I have been left stranded without API documentation, I look at method implementations to figure out which method does the thing I want, if any. This is normally at least somewhat painful--but it's occasionally downright pleasant, when the author of the original code has written it in this "every method is written as if it is exactly as deep in the abstraction hierarchy as you'll need to go" style. Which is to say, when you code like this, it's pleasant enough to read the source for that purpose that docs become somewhat redundant. I could imagine an editor designed to interact with code written like this, just displaying in a sidebar the implementation of any function you scroll past in autocomplete--because that's exactly as much, and as little, as you need to know to decide whether that function is the one you want.

59 points by magnars 14y ago

I agree that unit tests for internal (non public) functionality are not particularly useful, and can hamper your willingness to change an internal implementation for the better. I also agree it's painful to trace through dozens of classes/methods to determine what the actual implementation is. However, I don't agree the culprit is small methods. Small, interdependent methods in a single class (or package) are great and can be easy to follow if they are consistent. Usually the culprit is overuse of inheritance and design patterns. However, one case where I find myself being less "fancy" is functions that run/generate SQL statements. Although there are many cases where you can re-use certain portions of SQL statements and build up a query through smaller functions, this ends up being a nightmare to debug. I find it is better to be as plain and explicit as possible.

nnq 14y ago

on CleanCoder: I know $1 is just $1... but putting a sample video that I can just click to see would help a lot

BruceIV 14y ago

Hear, hear - I've been marking undergrad coding assignments all term, and the ones where they make a new method for absolutely every block of their code are nearly as unreadable as the ones that don't know how to indent or pick good variable names.

d0m 14y ago
Mind to show an example? Does the new method have a meaningful name? Are they smartly structured in a hierarchical order? I find it hard to believe that too much methods are harder to read than bad variables or indentation.
- BruceIV 14y ago
  Keep in mind that these are undergrad projects, so a few hundred LOC tops. But you'll see something like: bool noMoreElves() { return elfCount == 0; } void fireAllReindeer() { for (int i = 0; i < reindeerCount; ++i) { delete reindeerArray[i]; } delete reindeerArray; } void checkCloseWorkshop() { if ( noMoreElves() ) { fireAllReindeer(); } } the above is certainly readable, but takes forever to parse -- the below is, I would argue, just as clear, and much quicker to read (the point the OP made) void checkCloseWorkshop() { if ( elfCount == 0 ) { // fire all reindeer for (int i = 0; i < reindeerCount; ++i) { delete reindeerArray[i]; } delete reindeerArray; } } the comment above the loop is possibly superfluous, but helpful
  - skywalk 14y ago
    That goes against many recommendations I've read about refactoring and writing cleaner code: "Good code invariably has small methods and small objects. I get lots of resistance to this idea, especially from experienced developers, but no one thing I do to systems provides as much help as breaking it into more pieces." – Kent Beck
    - dalke 14y ago
      The little research I've seen has not backed up that opinion. While that view is shared by many - especially those who come from a Smalltalk heritage - the studies I've read about do not show a clear association between small pieces and more readable/maintainable code, at least not for code blocks under 100 lines or so. I dislike reading Smalltalk influenced code with many small pieces. They tend to use instance state as a way to pass parameters, and that makes me uncomfortable - I have a leaning towards functional programming. I also distrust function names. A function like "noMoreElves" might actually be "no more elves!" and implemented as "elfCount = 100". I exaggerate a little, but an inline "elfCount == 0" is much easier to verify than having to jump elsewhere to see the code.
    - ramblerman 14y ago
      http://stackoverflow.com/questions/153234/how-deep-are-your-unit-tests/153565#153565 http://stackoverflow.com/questions/153234/how-deep-are-your-... Kent beck also gave this amazing (and suprising) answer on SO about "how far to go with your tests?" He is in my eyes, a true pragmatist and gives some sound advice. Uncle bob on the other hand seems to have drunk too much of his own koolaid. I saw him give a talk 2 years ago at a java conference which made him seem more like a Steven Balmeresque salestype, not an active developer.

dexen 14y ago

> Did you ever learn how to read code at university? Were you shown loads of different code, in different languages and different styles, and then asked to explain what the code did? I don’t think code comprehension is a normal part of the curriculum for computing science classes. We’re taught how to write, not how to read. Maybe that’s a mistake. Dear lazyweb, I want to bridge that gap in my education. Where do I go to read loads of code in different styles? With the emphasis on plurality of styles.

indiecore 14y ago
Github?
- robertfw 14y ago
  I would recommend the above, along with chapters from http://www.aosabook.org/en/pypy.html http://www.aosabook.org/en/pypy.html as a guide
dalke 14y ago
I'll augment indiecore's answer. What kind of code are you interested in? Suppose you are, like me, interested in cheminformatics software. Find a few different packages (Open Babel, RDKit, Indigo, and others), download them, and compare how they implement a given task, like SMILES parsing or fingerprint generation. If you're interested in text compression, then download a few compression toolkits and see how they work. Also read a few articles on the topic. Regular expression engines? Dozens are available. Interested in networking? Take a look at ZeroMQ and other networking packages. Databases? That's a big topic, but you don't need to understand the entire package. Pick some aspect which interests you and see if you can figure out how SQLite, PostgreSQL, and MySQL handle it. Here's an easy one to start off with - how do Lua, Python, and C++ implement hash table-like data structures?
aidenn0 14y ago
Reading code without a purpose can cause your brain to tune out. You think you're learning but you aren't. I would rather suggest you find an open-source project and fix a bug in it. You will have to read lots of code in order to do so, and, so long as there is a repeatable test-case, fixing the bug shouldn't be too hard.

aidenn0 14y ago

I'm not sure I agree with this. If I'm looking at the implementation of a function, it's because it's done something wrong. For this function, I would fire up the debugger and say: "Does isTestPage() return what I expect? Yup, so the problem is going to be in surroundPageWithSetupsAndTeardowns()." A well named function becomes a black-box; you can tell if it's working just based on its inputs and outputs. Why open black boxes that are working?

thibauts 14y ago
> Why open black boxes that are working? Maybe to modify them ?
- el_cuadrado 14y ago
  Well, modifying a distinct method is much easier than modifying something in a middle of 20-page scroll of code. There are simply fewer thing that can go wrong.
- klibertp 14y ago
  Or even to understand how they are working and to check if they are really working correctly? I really don't like this attitude of ignoring inner workings of things just because they seem to work. Sometimes we really are working under constraints that make digging through the code impractical, but in my experience these constraints tend to be in the code itself - namely, someone wrote it as a 'black box' with tons of kludges and `if False:` style comments. I don't want to exaggerate and say that every programmer should know his every tool down to it's last bit, but taking black boxes apart is what makes one a hacker, and what allows one to improve one's skills.
  - aidenn0 14y ago
    Fair enough. Curiosity is a very reasonable excuse for opening a working black box, and is definitely what makes one a hacker. On the other hand, most systems these days are so complex that you have to have some black boxes. I know more about how my linux kernel works than the vast majority of people I work with, and yet I would classify the majority of it as a black box. I have a general idea of various trade-offs in efficiency and fragmentation resistance in malloc implementations, and if I have to I can dig into one. On the other hand, jemalloc works great and I just don't have the time to take it apart, so I just use it. Ultimately to get things done you need to trust that functions do what they say they do until you get evidence to the contrary. Taking functions apart can make you better at doing your job, but rarely fixes the problem you have Right Now.

guard-of-terra 14y ago

The problem with if (page.hasAttribute(“Test”)) {… is when you then decide to change boolean attribute Test to an enum attribute Environment with values Development, Test and Production. Because now you have to hunt down all the page.hasAttribute(“Test”) in your code and replace them with something different. And if your code is creepy enough you will miss one or two and something will break. So you have to look very carefully whether you repeat the same checks. If you repeat a check in different places and it has some meaning, you should abstract it away.

engtech 14y ago
The bigger problem I had with that snippet is the exposed string. I haven't seen the original video this is from, but I can't wonder but removing this hardcoded string was the bigger reason. Encoded strings like this are a maintenance problem lying in wait. How do you grep/search for it? These are all equivalent: if (page.hasAttribute("Test")) {… if (page.hasAttribute('Test')) {… string test = "Test"; if (page.hasAttribute(test)) {… string cheese = "Test"; if (page.hasAttribute(cheese)) {…
hapless 14y ago
The core problem is that it moves part of your OO interface into data. With "isTestPage()" you have a method with well-specified outputs. With "page.hasAttribute('Test')," a load of things become part of the permanent interface of the class: hasAttribute(), the datatype of 'Test,' the content of the field in the constructed argument etc...
anonymous 14y ago
That's simple - change the method so all those invocations are invalid now. If you're using a compiled language, the compiler will tell you all the lines that need changing. If you're using a dynamic language, you should have test coverage for all those methods and your tests will blow up with "no such method". Or maybe, you could special-case the 'hasAttribute(name)' method so it logs a debug warning and checks the enum attribute. Really, there are enough ways to deal with that and you're far from the first person to have had that problem.

beatgammit 14y ago

Is Alt+Shift+M significant to the article? I don't normally code in IDEs.

tltjr 14y ago
Alt+Shift+M extracts text to a method in Eclipse.

teamonkey 14y ago

Programming like a pirate? I was expecting this to be about R.

killnine 14y ago

When you look at the table of contents and think chapter 5 looks interesting, you don’t want chapter 5 to simply be another table of contents - which in turn links to a third table of contents etc yes!

pnathan 14y ago

This is part of the art of our work as software craftspeople; choosing when to refactor and when not to. There's no great joy in reading 400-line functions, and no great joy in hunting down 10 levels of the call stack to find out what is going on.

derefr 14y ago

> Think about this, in your experience - why do go have a look at the implementation of a method? Either you need to change it somehow, or it isn’t behaving as you thought it would. When I have been left stranded without API documentation, I look at method implementations to figure out which method does the thing I want, if any. This is normally at least somewhat painful--but it's occasionally downright pleasant, when the author of the original code has written it in this "every method is written as if it is exactly as deep in the abstraction hierarchy as you'll need to go" style. Which is to say, when you code like this, it's pleasant enough to read the source for that purpose that docs become somewhat redundant. I could imagine an editor designed to interact with code written like this, just displaying in a sidebar the implementation of any function you scroll past in autocomplete--because that's exactly as much, and as little, as you need to know to decide whether that function is the one you want.