> This is actually really bad practice and a very “over eager junior engineer” way of writing software.

To recycle a brief analysis [0] of my own youthful mistakes:

> I used to think I could make a wonderful work of art which everyone will appreciate for the ages, crafted so that every contingency is planned for, every need met... But nobody predicts future needs that well. Someday whatever I make is going to be That Stupid Thing to somebody, and they're going to be justified demolishing the whole mess, no matter how proud I may feel about it now.

> So instead, put effort into making it easy to remove. This often ends up reducing coupling, but--crucially--it's not the same as some enthusiastic young developer trying to decouple all the things through a meta-configurable framework. Sometimes a tight coupling is better when it's easier to reason about. [...]

[0] https://news.ycombinator.com/item?id=41219130

I saw some code in a job I was just starting where they had added several abstractions that I found...confusing.

After taking an extra long time to understand what the code actually did, I realized that some junior engineer had been using some design pattern they didn't really understand, and that added zero actual value to the routine.

After deleting all of that code and refactoring it to use completely different abstractions, everything was suddenly much easier to read and to extend.

Design is a hard skill to learn, and junior developers profoundly haven't learned that skill yet. But that's what we need to teach them as senior engineers, right?

Not that I could teach the author of the code I changed, since I think it was written by an intern that no longer worked for the company. But you do what you can.

There is also the fact it’s much easier to write something when you know where you are going. When you start you often just make lots of items general in nature to improve later on.

I mean... some do, some don't. With experience comes appreciation for simplicity and flexibility.

yep. good judgment comes from experience. experience comes from bad judgment.

It's easier to know where you're going when you've gone to similar places a hundred times.

I just think about a problem for two seconds and then have the entire path mapped out.

As someone in leadership, my ‘strong opinion held loosely’ on this, is that there’s absolutely no way to meaningfully build this skill in people, in a theoretical setting.

You can, at best, make them aware that there is such thing as “too much”, and “the right tool for the job”, and keep reminding them.

But nothing, nothing, comes remotely close to the real-world experience of needing to work with over-engineered spaghetti, and getting frustrated by it. Especially if it’s code that you wrote 6 months prior.

Juniors will always do this. It’ll always be the senior’s job to…let it happen, so the junior learns, but to still reduce the blast radius to a manageable amount, and, at the right moment, nudge the junior toward seeing the errors in their ways.

>nudge the junior toward seeing the errors in their ways.

At least plant a seed of doubt early on.

> I realized that some junior engineer had been using some design pattern they didn't really understand, and that added zero actual value to the routine.

£3.50p says it was the Generic Repository pattern implemented over Entity Framework dbContext, right?

--------

Speaking of design-patterns, I subscribe to the opinon that _Design-patterns are idioms to work-around missing features in your programmign language_, which explains why Java has no end of them, and why us jaded folk find happiness in modern languages that adopt more multi-paradigm and FP (the post-Java cool-kids' club: Kotlin, Rust, Swift, TypeScript, (can C# join?)) - so my hope is that eventually we'll have a cohort of fresh-faced CS grads entering industry who only know-of Facades/Decorator/Adapter as something a language designer does to spite their users because any reasonable compiler should handle interface-mapping for you - and the Visitor-pattern as a great way to get RSI.

FP has design patterns too, just different ones, and they don't all have tidy names.

Also some GoF design patterns map pretty closely to FP equivalents... pattern-matching on ADTs + traverse/fold + ReaderT ends up looking a lot like the visitor pattern.

> pattern-matching on ADTs + traverse/fold + ReaderT ends up looking a lot like the visitor pattern.

...that sounds like hand-written tedium; isn't `replicate` meant to avoid that?

Nah, it was something unrelated to databases.

I can't even remember the details. It's like trying to remember nonsense sentences; they don't stick because they don't really make sense.

To the best I can remember, it was something like the use of an adapter pattern in a class that was never going to have more than one implementation? And it was buried a couple layers deep for no particularly good reason. Or something.

And yes, modern languages like the ones you list make many of the original GoF Design Patterns either absolutely trivial (reducing them to idioms rather than patterns) or completely obsolete.

when I was learning Go, I read a guide that told you to fire off a goroutine to walk a tree and send the values back to the main goroutine via a channel. I think about that "just an example" guide a lot when I see bad channel code.

For me the biggest red flag is somebody using a channel as part of an exported library function signature, either as a param or a return value. Almost never the right call.

At the time, that was one of the only ways to write decent-looking generic code.

I've used that pattern to write tools to e.g. re-encrypt all whatever millions of objects in an S3 bucket, and examine 400m files for jars that are or contain the log4j vulnerable code. I had a large machine near the bucket/NFS filer in question, and wanted to use all the CPUs. It worked well for that purpose. The API is you provide callbacks for each depth of the tree, and that callback was given an array of channels and some current object to examine; your CB would figure out if that object (could be S3 path, object, version, directory, file, jar inside a jar, whatever) met the criteria for whatever action at hand, or if it generated more objects for the tree. I was able to do stuff in like 8 hours when AWS support was promising 10 days. And deleted the bad log4j jar few times a day while we tracked down the repos/code still putting it back on the NFS filer.

The library is called "go-treewalk" :) The data of course never ends back in main, it's for doing things or maybe printing out data, not doing more calcualation across the tree.

The only time I've seen it work with channels in the API is when it's something you'd realistically want to be async (say, some sort of heavy computation, network request, etc). The kind of thing that would probably already be a future/promise/etc in other languages.

And it doesn't really color the function because you can trivially make it sync again.

> And it doesn't really color the function because you can trivially make it sync again.

Yes, but this goes both ways: You can trivially make the sync function async (assuming it's documented as safe for concurrent use).

So I would argue that the sync API design is simpler and more natural. Callers can easily set up their own goroutine and channels around the function call if they need or want that. But if they don't need or want that, everything is simpler and they don't even need to think about channels.

> The kind of thing that would probably already be a future/promise/etc in other languages.

Or a coroutine (caller calls `yield(item)` for each match, and `return` when done).

> when I was learning Go, I read a guide that told you to fire off a goroutine to walk a tree and send the values back to the main goroutine via a channel.

Okay, I gotta ask - what exactly is wrong with this approach? Unless you're starting only a single goroutine[1], this seems to me like a reasonable approach.

Think about recursively finding all files in a directory that match a particular filter, and then performing some action on the matches. It's better to start a goroutine that sends each match to the caller via a channel so that as each file is found the caller can process them while the searcher is still finding more matches.

The alternatives are:

1. No async searching, the tree-walker simply collects all the results into a list, and returns the one big list when it is done, at which point the caller will start processing the list.

2. Depending on which language you are using, maybe have actual coroutines, so that the caller can re-call the callee continuously until it gets no result, while the callee can call `yield(result)` for each result.

Both of those seem like poor choices in Go.

[1] And even then, there are some cases where you'd actually want the tree-walking to be asynchronous, so starting a single goroutine so you can do other stuff while talking the tree is a reasonable approach.

> what exactly is wrong with this approach?

Before Go had iterators, you either had callbacks or channels to decompose work.

If you have a lot of files on a local ssd, and you're doing nothing interesting with the tree entries, it's a lot of work for no payoff. You're better off just passing a callback function.

If you're walking an NFS directory hierarchy and the computation on each entry is substantial then there's value in it because you can run computations while waiting on the potentially slow network to return results.

In the case of the callback, it is a janky interface because you would need to partially apply the function you want to do the work or pass a method on a custom struct that holds state you're trying to accumulate.

Now that iterators are becoming a part of the language ecosystem, one can use an iterator to decompose the walking and the computation without the jank of a partially applied callback and without the overhead of a channel.

Assuming latest Go 1.13 I would write an iterator and used goroutines internally.

The caller would do:

    for f := range asyncDirIter(dir) {
    }

Better than exposing channel.

But my first question would be: is it really necessary? Are you really scanning such large directories to make async dir traversal beneficial?

I actually did that once but that that was for a program that scanned the whole drive. I wouldn't do it for scanning a local drive with 100 files.

Finally, you re-defined "traversing a tree" into "traversing a filesystem".

I assume that the post you're responding to was talking about traversing a tree structure in memory. In that context using goroutines is an overkill. Harder to implement, harder to use and slower.

> In that context using goroutines is an overkill. Harder to implement, harder to use and slower.

I agree with this, but my assumption was very different to yours: that the tree was sufficiently large and/or the processing was sufficiently long to make the caller wait unreasonably long while walking the tree.

For scanning < 1000 files on the local filesystem, I'd probably just scan it and return a list populated by a predicate function.

For even 20 files on a network filesystem, I'd make it async.

I get your point, but a wait group or a mutex can be removed in favor of a clean usage of channels if the proper concerns are isolated at first. And I would personally much rather reason about channels than mutexes and wait groups. Wait groups and mutexes are just begging for deadlocks and race conditions, where a proper channel, used correctly, eliminates both of those by design.

> Wait groups and mutexes are just begging for deadlocks and race conditions, where a proper channel, used correctly, eliminates both of those by design.

By that same logic, if you just use wait groups and mutexes correctly, you should also not worry about deadlocks and race conditions. It's also quite trivial to introduce a deadlock with a channel.

Regardless, channels are basically a more expressive/flexible type than mutexes, waitgroups, and function calling, but in the same family as all of them. You can implement any of those rather trivially with a channel, but there are things you can do with a channel that are quite complex or impossible to implement using those. Such a flexible tool allows you to start doing things that are "easy" to implement yet poor design decisions. For example, instead of direct function calling you can now start passing data over channels, which "works" just as well except it incurs some scheduling overhead (not always a concern depending on how perf sensitive you are), makes debugging and interpreting stack traces more difficult (increasingly so as the logic on both sides of the channel increases over time), and allows the software to start evolving in an unintended way (specifically into an overly complex Actor model with tons of message-passing that is impossible to untangle, rather than a directed tree of direct function calling). Or you have a hard time understanding the state and properties of a piece of data throughout the program lifetime because it doesn't "belong" anywhere in particular.

---

Something I thought about recently: perhaps the biggest balancing act in software is between specificity and expressiveness. To solve a problem you might be able to find something that is perfectly tailored to your needs where you just click something or run something and your problem is solved, but it's highly likely that it will only solve that specific problem and not others ones. Alternatively, a lot of software (like Jira, SAP, many enterprise software) is highly configurable and expressive but requires a lot of effort to set up and may not be particularly good at solving your specific task. At its most extreme, you could technically call a computer containing only a brainfuck compiler and basic text editor as being able to solve any problem solvable by a computer, because it's a programmable turing machine.

This extends even into the weeds of programming, especially when you're working on software with other people or over long periods of time, where you might struggle to enforce or maintain your particular mental model for how the software should work. When faced with implementing something with an expressive approach vs a specific one, you want to be expressive enough to be able to modify the code later to do things you plan to do or think you have a high probability of doing, but you want to be specific enough that the purpose and function of something (be it a library, class, binary, or entire distributed software system) is clear - if it isn't clear, the people using it will struggle with it or avoid it, and the people working on it will start taking it in a direction you didn't intend.

Channels are the type of thing that are expressive enough to be broadly applicable, but are easily misinterpreted (you might be using them to implement parallelism, but your coworker Bob might think you're using them because you want to design your software under a message-passing actor model) and easily misused. They also make it very, very easy to "code yourself into a corner" by introducing inscrutable logical/data paths that can't be untangled. You might be able to use them safely in lieu of a mutex but it only takes one Bob to start taking them in the direction of unmaintainability. And sometimes you might be that Bob without knowing it. That's why I think it's best to avoid them unless your other options are even worse.

Using channels where mutexes would suffice has by far been the main cause of bad concurrent code I've encountered.

Using more than 2 'semantic' channels plus one ctx.Done() channel? There's probably a bug. So far that has been well over 50% accurate, across dozens of libraries.

When they're used like this, chans often break into non-blocking algorithm details, because they don't ensure mutual exclusion. And non-blocking algorithms are freakin hard - few things are guaranteed without great care.

"Do not communicate by sharing memory; instead, share memory by communicating." - https://go.dev/blog/codelab-share

Wait groups are preferred to channels for the purposes they serve. Mostly waiting for goroutines to finish. You can use a channel but wait groups are much cleaner.

Mutexes for shared memory are less preferred than channels. There are always exceptions.

But yeah, if all you have is a hammer then everything looks like a nail. Go has mutexes and wait groups and channels and all of these have their right place and use case. If you're using mutexes to effectively re-implement what channels support then you're doing it wrong. If you're using channels for something that can be a function call then you're also doing it wrong. Software is hard.

> By that same logic, if you just use wait groups and mutexes correctly, you should also not worry about deadlocks and race conditions.

I agree with most everything else you said - especially about software being a trade off between specificity and expressiveness - but I can’t agree with this.

The problem being that a mutex can hide things that a channel can’t. Channels will always give you what you expect, but that is not the case for mutexes or wait groups or error groups or whatever.

Honestly, the older I get, the more I understand that joke about “you must be this tall to write concurrent programs” and the mark is at the ceiling.

Back at uni, we had a 200-level ‘software engineering’ unit, largely introducing everyone to a variety of ‘patterns’. Reading the Gang of Four book, blah blah blah. You get the idea.

Our final assignment for this unit was to build a piece of software, following some provided specification, and to write some supplementary document justifying the patterns that we used. A mature-aged student that had a little bit of industry experience under his belt didn’t use a single pattern we learned about the entire semester. His code was much more simple as a result. He put less effort in, even when taking into account his prior experience. His justifying documentation simply said something to the effect of “when considering the overall complexity of this problem, and the circumstances under which this software is being written, I don’t see any net benefit to using any of the patterns we learned about”.

He got full marks. Not in a “I tricked the lecturer!” way. I was, and still am, a massive fan of the academic that ran the unit. The feedback the student received was very much “you are 100% correct, at the end of the day, I couldn’t come up with an assignment that didn’t involve an unreasonable amount of work and ALSO enough complexity to ever truly justify doing any of the stuff I’ve taught you”. All these years later, I still tell this story to my team. I think it’s such a compelling illustration of “everything in moderation”, and it’s fun enough to stick with people.

So Grug was right all this time ?

Joke apart this is a really interesting example. In a job interview I've been asked once if I ever regretted something I did and I couldn't quite word it on the spot, but definitely my first project included extra complexity just so that it "looked good" and in the end would have been more reliable had I kept it simple.

Nice with such teachers :-) What's a 200 level unit? It means it's "very advanced"?

In the multi-year series of blaming (us) juniors for every ill in the programming world, they now also get blamed for over-architecting.

I took the opportunity to share that quote with some others on our project because this is a pattern that we recognize from our boss and a few of the productive consultants in the past. Not juniors but people who have/had the weight to set the code style tone of the project and has lead to hours of all of us scratching our head when having to read the code.

But thanks for souring me on this whole thread.