3 ms·

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 nea

by 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.
  - Noughmad 14y ago
    Both of your examples are fine, as it is pretty clear what each method does. In such cases, the golden rule is "if you do it more than once, create a new function". I don't know what kind of things you program if you have 100-line functions with no code sharing.
  - qu4z-2 14y ago
    The main point is that when you realise you forgot to set reindeerCount = 0. If closing the workshop is the only situation in which you fire all the reindeer, it's probably fine. If you need to fire the reindeer in other situations, you now need to manually fix the code everywhere.
    - BruceIV 14y ago
      Thanks, good catch (my code probably would have been better if I had an actual use-case instead of a contrived example).
  - d0m 14y ago
    As far as I understand it, it's more of a "Bottom/Top" versus "Top/Bottom" thinking. Personally, looking at: void checkCloseWorkshop() { if ( noMoreElves() ) { fireAllReindeer(); } } is enough, I don't need to look more precisely. But, for someone who is more "Bottom/Top", the second version is a headache because they need to jump everywhere and keep 10 different files/functions open all the time. An anecdote of this, I once worked with a programmer who was "Bottom/Top" and really hated functions. (1500+ lines function was the way to go for him). He would then refactor my code like this: void checkCloseWorkshop() { bool noMoreElves() { return elfCount == 0; } void fireAllReindeer() { for (int i = 0; i < reindeerCount; ++i) { delete reindeerArray[i]; } delete reindeerArray; } if ( noMoreElves() ) { fireAllReindeer(); } } I.e. Pasting my functions into the other functions he needed.. rather than just calling mine or moving them in another file. Obviously, there was no tests or frameworks used by this company. This is a true example of "Bottom/Top" thinking. Last note, I understand your point but there's a way to mix both strategies by using high-level strategies. For instance, fireAllReindeer is deleting an array while noMoreElves is checking for emptiness. Good C++ programmers would use boost or the STD functors/SmartPointers rather then manually reinventing the wheel every time.
- BruceIV 14y ago
  To more directly answer your question, it's more a problem of needing to remember an unnecessarily large number of names - in addition to the class variables numberOfElves and reindeerArray from my example, I now need to keep track of twice as many names. Also, admittedly, after the first 5 or 10 repetitions of the same assignment, I know what all the valid solutions look like, so obfuscating the code by putting little chunks of code in separate methods makes it harder to read (but that's a readability problem specific to marking assignments).
  - anonymous 14y ago
    "a large number of names" is a metric that differs for different people. Some find it much easier to parse text when everything is named, some prefer to have things unnamed and just spelled out. Think of having a hash from name->function vs a hash essence->function. The first people can see a name, remember it, look it up and form a connection. The latter can see a logical block of code, parse it and then recognise it when they see it again. Personally, I'm in the latter camp. There are people whose names I've been told more than 5 times, but still don't remember them. I remember the person, but not the name. In school, I almost never knew any teacher's name and it took me several years to learn the names of most of the other children. This also means that you can't have a "One True Most Readable Style". You only have several local maximums for different classes of people.

by BruceIV 14y ago

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.
- Noughmad 14y ago
  Both of your examples are fine, as it is pretty clear what each method does. In such cases, the golden rule is "if you do it more than once, create a new function". I don't know what kind of things you program if you have 100-line functions with no code sharing.
- qu4z-2 14y ago
  The main point is that when you realise you forgot to set reindeerCount = 0. If closing the workshop is the only situation in which you fire all the reindeer, it's probably fine. If you need to fire the reindeer in other situations, you now need to manually fix the code everywhere.
  - BruceIV 14y ago
    Thanks, good catch (my code probably would have been better if I had an actual use-case instead of a contrived example).
- d0m 14y ago
  As far as I understand it, it's more of a "Bottom/Top" versus "Top/Bottom" thinking. Personally, looking at: void checkCloseWorkshop() { if ( noMoreElves() ) { fireAllReindeer(); } } is enough, I don't need to look more precisely. But, for someone who is more "Bottom/Top", the second version is a headache because they need to jump everywhere and keep 10 different files/functions open all the time. An anecdote of this, I once worked with a programmer who was "Bottom/Top" and really hated functions. (1500+ lines function was the way to go for him). He would then refactor my code like this: void checkCloseWorkshop() { bool noMoreElves() { return elfCount == 0; } void fireAllReindeer() { for (int i = 0; i < reindeerCount; ++i) { delete reindeerArray[i]; } delete reindeerArray; } if ( noMoreElves() ) { fireAllReindeer(); } } I.e. Pasting my functions into the other functions he needed.. rather than just calling mine or moving them in another file. Obviously, there was no tests or frameworks used by this company. This is a true example of "Bottom/Top" thinking. Last note, I understand your point but there's a way to mix both strategies by using high-level strategies. For instance, fireAllReindeer is deleting an array while noMoreElves is checking for emptiness. Good C++ programmers would use boost or the STD functors/SmartPointers rather then manually reinventing the wheel every time.
BruceIV 14y ago
To more directly answer your question, it's more a problem of needing to remember an unnecessarily large number of names - in addition to the class variables numberOfElves and reindeerArray from my example, I now need to keep track of twice as many names. Also, admittedly, after the first 5 or 10 repetitions of the same assignment, I know what all the valid solutions look like, so obfuscating the code by putting little chunks of code in separate methods makes it harder to read (but that's a readability problem specific to marking assignments).
- anonymous 14y ago
  "a large number of names" is a metric that differs for different people. Some find it much easier to parse text when everything is named, some prefer to have things unnamed and just spelled out. Think of having a hash from name->function vs a hash essence->function. The first people can see a name, remember it, look it up and form a connection. The latter can see a logical block of code, parse it and then recognise it when they see it again. Personally, I'm in the latter camp. There are people whose names I've been told more than 5 times, but still don't remember them. I remember the person, but not the name. In school, I almost never knew any teacher's name and it took me several years to learn the names of most of the other children. This also means that you can't have a "One True Most Readable Style". You only have several local maximums for different classes of people.