last day (16 days later) » 

03:52
104
A: My boss asks me to stop writing small functions and do everything in the same loop

David ArnoTaking the code examples first. You favour: if (isApplicationInProduction(headers)) { phoneNumber = headers.resourceId; } else { phoneNumber = DEV_PHONE_NUMBER; } function isApplicationInProduction(headers) { return _.has(headers, 'resourceId'); } And your boss would write it as: // T...

disagree.. creating a method to to do something so simple is useless unless it is being reused all over the place.
@MatthewWhited Unless 4 weeks later you go back to it and wonder why your condition is structured like that, because the code itself doesn't tell that without knowing much broader, business context.
it's clear what it is doing. if you are concerned then add a comment to say what it is doing not how or why.
@MatthewWhited, "f you are concerned then add a comment to say what it is doing". As I say in my answer, I find it utterly incomprehensible why anyone would ever consider replacing a function name, that describes what the code does, with inline code accompanied by a comment that descries what the code does.
many of us perfer to see the inline code and have no problem understanding it. If you were going to make a function it should be sometihng like getPhoneNumber(...) not that isInProduction... with and still user the ternary
03:52
A loop should fit one screen page. That gives you a rough overview at a glance. You will then go into details in the next step. 100 lines would not fit one page (my eyes are too old and the screen resolution too). Every time you need to scroll, it starts messing up your mind. In former times we had printouts from the line printer. The two pages are still about the same that fits one screen page.
@MatthewWhited, OK, so to you find inline code with comments easier to read. Makes no sense to me, but each to their own. And you find this easier to unit test too? And refactor without breaking other tests?
@ThomasKilian, "A loop should fit one screen page". What?!?? Not in my world, mate. I aim for the whole file to fit on the screen, and set myself a non-negotiable (except in real edge-case situations) 200 line limit to files and 20 lines for functions. Never looked back since doing that.
Loop is probably the wrong word. It means "complex structure to be refined". Starts with the whole thing one needs to break down. Each break down should fit one page.
than methods I have to chase down... yes, inline code is easier.
@ThomasKilian, makes sense now. I can definitely relate to the "my eyes are too old" bit. :)
+1 for "interested in the 'how', only the 'what' of a piece of functionality." Also, regarding testing: forget unit testing; comments can't be tested at all.
03:52
Code Complete also concurs with Clean Code - Focus on writing code at a single level of abstraction within a function. If you find yourself moving up and down the context ladder, it's a good indication things need to be refactored.
@MatthewWhited Having it inline is all well and good until you hit an edge case and add handling for that. And then another edge case. And another. There's no detriment to adding a function while the implementation is "simple", and when the implementation becomes more complex, you have a clearly defined area where you add changes, and the code on the calling level gets to remain the same.
"I aim for the whole file to fit on the screen, and set myself a non-negotiable [...] 200 line limit to files" how can you fit 200 lines on your screen? Do you use the monitor in portrait mode?
@null, no, I just didn't explain it well. If I can keep a file to a few lines, so it fits on the screen, I'm happiest. But, that rarely happens, so I set a more practical 200 line limit. Hopefully makes more sense now.
@MathiasEttinger, oh that's brilliant :)
refactor later guys... holy crap. you don't need to create several methods to handle 1 line of non-duplicated code.
03:52
What is this ternary expression of which you speak? Perhaps you mean a conditional expression.
@DavidArno Some of my C++ code has 15-20 includes (each which have to be on their own lines) and then the function is only about 20 lines. So the file doesn't fit on the screen unless you scroll past the includes, but that implies that each of your files only has a couple functions. Do you keep more than one file open at the same time?
I disagree with the entire notion that a function "isApplicationInProduction" should rely on a parameter called "headers". Surely it's more fundamental than which headers you happen to have around right now.
@andy256, well if you want to be truly pedantic, then I clearly meant use the ternary operator to express that conditional expression. But life is too short for such pedantry: ternary expression works just as well.
@David Sorry, but programming is about precision. Call it pedantic if you wish, but sloppy language betrays sloppy thinking, and sloppy thinking causes software bugs, project overruns, cost overruns, and the like. Software is either right, or not. Life is too short to waste on building incorrect software.
@andy256, programming is primarily about human-to-human communication. That never involves precision and pedantry is one of the biggest inhibitors to that communication.
03:52
@David With the level of experience you have, your opinion would certainly be valued by many. From my 45 years experience, I have formed a different opinion: that fuzzy communication leads to fuzzy results, that are unlikely to satisfy clients. I suggest we leave it there.
@andy OP's code is already a conditional expression. If you hold to the precision you claim to demand you should ask for the conditional operator.
@DavidArno 200 lines max for a file? That means you're wasting a lot of time switching between files.
@LorenPechtel they're both really fast so it hardly matters, but for me switching files is precious milliseconds faster than scrolling 200 lines. But everyone has their own workflow.
@LorenPechtel, Like Mark, my experience is that eg switching between 5, 200 line files is faster (I would say a lot faster, rather than just milliseconds) than scolling through a 1,000 line file.
@MathiasEttinger the same account tweets "We don't pay you to think, we pay you to code" ..I'd take their words with a tiny little grain of salt.
03:52
@Mat'sMug, um, it's a parody account. That's the whole point to that account. It ridicules "expert beginner" stupid ideas, like those of the OP's boss. "there's a 15 LOC minimum on all methods" is as much an anti-pattern as "We don't pay you to think, we pay you to code".
Oh... I misread the author of the comment, thought it was MatthewWhited. Too many Mats around here :-)
@Mat'sMug, Yep, far too many Mats :)
The real question is why you have a function which only performs a function. isApplicationInProduction(){} only returns the value of .has(headers, 'resourceId'), which makes me question why 'isApp...' even exists. Just use the function you're using, which is '.has'. Am I missing something?
@EvSunWoodard, "The real question is why you have a function which only performs a function". You might want to rethink and re-ask that question as I'm seriously struggling to think of an occasion when I'd want to have a function that did more than perform a function (maybe make a cup of tea, too?)
@David Arno, I meant only that the function 'isApp...' does not do any transformation of data, data storage, nor does it impact the overall program in any way. All 'isApp...' does is call the function '.has(...', so I am confused why you would abstract a one line function into another one line function with a different name, when you could just use '.has(...' in the first place.
03:52
@EvSunWoodard Because eventually that function can be replaced with some code that actually performs a meaningful test for a production environment, but the rest of the code doesn't need to be altered.
"If the function is well-named" and this is typically where the pain lies.
@EvSunWoodard If you're a computer, the isApp... function doesn't impact the overall program in any way. If you're a human, the isApp... function does two things: 1) tells you why there are two phone number branches (one's for production, the other's not) 2) why we're looking up the resourceId header (to see if we're in production).

  last day (16 days later) »