4 ms·
This is actually really bad practice and a very “over eager junior engineer” way of writing software. You’re not off base at all that it seems excessive and con
by weitendorf 2y ago
This is actually really bad practice and a very “over eager junior engineer” way of writing software. You’re not off base at all that it seems excessive and confusing. It’s the kind of thing that seems technically complex and maybe even “elegant” (in isolation, when you first write the “interesting” code) at first but becomes a technical nightmare when used in real software that has to grow around and with it. You’re actually more on point in worrying about the understandability and debuggability this introduces.
I spent the better part of two years unfucking some Go software that (among other things) misused channels. The problem with channels is that you rarely actually need them, but can use them for a lot of different things without too much initial difficulty.
I think a good litmus test for proper use of channels is if you answer no to “could this be done with a direct function call instead?” and “can I use a wait group or mutex instead”, and yes to (zooming out a bit to think about what kind of decisions you previously made that led you to think about using channels) “am I really benefitting from concurrency/parallelism enough to justify the technical complexity of debugging concurrent code”.
- zachmu 2y agowhen 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.
- lanstin 2y agoI'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.
- Filligree 2y agoAt the time, that was one of the only ways to write decent-looking generic code.
- deleted 2y ago[deleted]
- Kamq 2y agoThe 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.
- beautron 2y ago> 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.
- lelanthran 2y ago> 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).
- lelanthran 2y ago> 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.
- fnord123 2y ago> 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.
- kjksf 2y agoAssuming 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.
- SomeCallMeTim 2y agoI 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.
- RandomWorker 2y agoThere 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.
- SonOfLilit 2y agoI mean... some do, some don't. With experience comes appreciation for simplicity and flexibility.
- vram22 2y agoyep. good judgment comes from experience. experience comes from bad judgment.
- SomeCallMeTim 2y agoIt'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.
- cqqxo4zV46cp 2y agoAs 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.
- Terr_ 2y ago> 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 https://news.ycombinator.com/item?id=41219130
- distortionfield 2y agoI 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.
- weitendorf 2y ago> 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.
- cqqxo4zV46cp 2y agoBack 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.
- bionsystem 2y agoSo 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.
- cutemonster 2y agoNice with such teachers :-) What's a 200 level unit? It means it's "very advanced"?
- keybored 2y agoIn 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.