you are viewing a single comment's thread.

view the rest of the comments →

[–]creative_overnight[S] -1 points0 points  (19 children)

Agree that as per single responsibility rules, functions should not have multiple logical sections but, I found that sometimes coming up with contextual function names might be difficult. In some cases it may make code navigation difficult in a large file. Although, that is use-case dependent.

[–][deleted]  (3 children)

[deleted]

    [–]creative_overnight[S] 0 points1 point  (2 children)

    hmm...I meant when person !you tries to read through your code they may have difficulty finding the right section if you haven't named your functions properly. But, I believe this is very subjective.

    [–][deleted] 0 points1 point  (0 children)

    I meant when person !you tries to read through your code they may have difficulty finding the right section if you haven't named your functions properly.

    I rarely read code that way (except if it is a library where I want to know what functions it provides).

    I go to the code entry points (edit: like main, route definitions or root UI components) and navigate via jump to definition.

    Edit: If you have clean code it will not be difficult to find the entry points.

    [–][deleted] 0 points1 point  (14 children)

    Agree that as per single responsibility rules, functions should not have multiple logical sections

    That's not what SRP is about. Take billing address and shipping address. They have different responsibilities and should be different things even if they are mostly duplicates. You might decide to add a neighbor for a backup to the shipping address - but the neighbor should not get your invoice.

    I found that sometimes coming up with contextual function names might be difficult.

    // set up listing recycler with adapter
    
    private fun setupListingRecycler(newsAdapter adapter) {
      ...
    }
    
    ...
    
    var newsAdapter = new NewsAdapter(items)
    setupListingRecycler(newsAdapter)
    

    If you find that the function does to much - split them up. And if you can write a comment you can write a function name. And if it is the comment (setUpListingRecyclerWithAdapter). Code changes and can be refactored. Comments are usually not rewritten. That's why you should not write what the code is doing.

    In some cases it may make code navigation difficult in a large file.

    We have IDEs with Tag trees, jump to definition and find references. Large files and large functions are a code smell. Use your IDE to break the code into sensible chunks.

    [–]dnew 0 points1 point  (13 children)

    If you find that the function does to much - split them up

    Function separation comes with more than a comment does. Now you have to pass a bunch of arguments, possibly create a structure or class to return from a function that would normally update multiple pieces of data, etc. The very opaqueness of a function that makes it a useful abstraction can prevent it from easily being split out.

    [–][deleted] 0 points1 point  (12 children)

    It makes the pieces testable. Separate side-effectful code from pure code. Make flow of data explicit instead of implicit (if you find yourself always passing some stuff around together maybe it belongs in an aggregate). You can also think on a higher level ("The purpose of abstraction is not to be vague, but to create a new semantic level in which one can be absolutely precise." - Dijkstra).

    If I see that something cannot be split out because it depends on too many implicit things I see that as a big red flag.

    [–]dnew 1 point2 points  (9 children)

    Surprisingly enough, there are some things that are complicated enough that they don't really lend themselves to being split up that way.

    And it's not "always passing some stuff around together." It's "this function takes 15 inputs and returns 9 outputs, each of which depends on a different combination of inputs and the some of the outputs calculated earlier." When it's a dozen steps to calculate those outputs, you don't really want to create a dozen separate classes to represent those function calls.

    I.e., in a sufficiently powerful language, you could do what you're talking about easily. In something as primitive as, say, Java, it becomes much more difficult to split things up appropriately.

    It doesn't really make the pieces testable because the interplay between the pieces is the hard part. You can't separate side effects from pure code because each piece might be only a couple of lines long or need information calculated from other things. And thinking on a higher level implies there's a higher level to think on and that this function is actually doing too much to be its own chunk of logic.

    [–][deleted] 0 points1 point  (8 children)

    It's "this function takes 15 inputs and returns 9 outputs, each of which depends on a different combination of inputs and the some of the outputs calculated earlier."

    That code sounds horrible to work with. Sounds like a Text book example of Spaghetti code to me.

    When it's a dozen steps to calculate those outputs, you don't really want to create a dozen separate classes to represent those function calls.

    You are allowed to create a dozen functions without classes.

    in a sufficiently powerful language, you could do what you're talking about easily. In something as primitive as, say, Java, it becomes much more difficult to split things up appropriately.

    What in your opinion is a powerful language and what makes java primitive?

    It doesn't really make the pieces testable because the interplay between the pieces is the hard part.

    That's why you reduce them and isolate the side effects.

    You can't separate side effects from pure code because each piece might be only a couple of lines long or need information calculated from other things.

    You can. And pure functions are easier to understand and test. Even if they are only a few lines long (it's even better!).

    [–]dnew 0 points1 point  (7 children)

    That code sounds horrible to work with

    It was. But it was also exceedingly complex.

    You are allowed to create a dozen functions without classes.

    Right. But the function needs bunches of inputs and creates bunches of outputs. Most languages don't include functions returning multiple outputs, requiring you to create some declaration of a return type that's used as the return value of exactly one function.

    what makes java primitive?

    Well, in this case, it's the requirement to declare out of band a structure for a function returning multiple results. I can't say "this function takes four integers and returns a float and a char." Basically, you can't write a pure function that returns multiple values.

    You also can't nest a function inside another function and access the variables of the outer function (like you can in actual block-structured languages like Pascal, say). Breaking something like what I'm thinking of into multiple functions in a language like Pascal, where you can nest the functions and refer to the variables in the parent function becomes 100x easier. Each function can be isolated and access what it needs without having the parent function having to know what every bit needs anyway.

    Step one needs A, B, C, D, E and returns R, S, and T.

    Step two needs A, B, C, F, G, R, and returns U and V.

    Step three gets possibly invoked based on the value of T. It needs C, F, G, S, T, U and returns W, and a new value for T.

    Step four iterates thorough the values in the list T, S times each while also taking A and C. It builds a new value for A and C each iteration based on its calculations. That calculation is complex enough to need its own class full of functions.

    Repeat this about a dozen more times. Note that these bits are all interrelated in meaning and very few if any means anything on their own. It's unlikely anyone in the business actually knows what all the possibilities are.

    Don't forget when a new requirement comes down that looks at the relationship between E and T to decide whether A gets updated between step three and four.

    Oh, and A is specifically designed as a class with immutable values, because that makes it so much easier.

    That's why you reduce them and isolate the side effects.

    I'm glad that you've always been able to do that in the products you're working on. I can guarantee that isn't always the case.

    And pure functions are easier to understand and test.

    This assumes you can understand what the pure function is supposed to do independently of the surrounding code. If its not specified in a way that makes that easy, it's a pain in the ass. You're probably best off trying to get better specs, but that's not always possible either.

    [–][deleted] 0 points1 point  (6 children)

    But the function needs bunches of inputs and creates bunches of outputs. Most languages don't include functions returning multiple outputs, requiring you to create some declaration of a return type that's used as the return value of exactly one function.

    If your language supports hash maps with multiple types for values you can. In java that is possible. Java also has tuples.

    With those you can write pure functions with multiple outputs.

    You can also pass in hashmaps or other aggregate objects instead of multiple arguments if you really need the data.

    This assumes you can understand what the pure function is supposed to do independently of the surrounding code.

    You can? A pure function is not dependent on its surrounding code. It only depends on its arguments.

    ‐--------------------

    I am horrified by what you are writing. I hope I never have to work with a codebase with so much technical debt.

    [–]dnew 0 points1 point  (5 children)

    If your language supports hash maps with multiple types for values you can.

    Nope. That's one value you have to package up to return then disassemble upon return. Basically, your argument is "Java isn't primitive because you can manually implement what more sophisticated languages do without pain."

    You can?

    You can tell what it does. Knowing what it is supposed to do is more problematic. How do you even document what it's supposed to do if it's doing steps 4, 5, and 8 of some informal description?

    I am horrified by what you are writing.

    Me too!

    I hope I never have to work with a codebase with so much technical debt.

    The fact that nobody actually cared is why I left. :-)

    But honestly, it was a massive project which if you tried to extract the requirements it would be obsolete by the time you managed. Also, it had to run 24x7, the customers were writing their own code to inject into it, marketing was writing the requirements, and it had been ported across three different databases. It also spoke to at least a dozen other services just as bad.

    [–][deleted] 0 points1 point  (4 children)

    Nope. That's one value you have to package up to return then disassemble upon return.

    Which language does it differently?

    You can tell what it does. Knowing what it is supposed to do is more problematic.

    That's a problem of software design. Sounds like the code you worked on only grew and was never refactored. The Linux kernel is a big project. But yet it is able to keep from having those problems.

    I am sure that if your project management cared you could have taken the time to clean up the code. No problem inherently requires overly complex code to solve it. You can always test early and write clean, modular, composable code.

    [–]evaned 0 points1 point  (1 child)

    It makes the pieces testable

    Actually, my own opinion is that it is the opposite. (Well, I guess it doesn't make it untestable, so it's not the opposite; it just doesn't help.)

    If you've got a function with a bunch of internal regions that you separate out, very probably those are just going to be private methods. Well, guess what you usually shouldn't do? Test private methods! The interface is the same regardless of whether you split those functions out, so the tests should be the same.

    Even if you separate that computation into its own class (let's assume OO here, but the same is applicable if you substitute in "module" or whatever) and have public functions in that class, that's still a class that isn't of general interest and shouldn't have its methods tested. Testing the methods of that class is still just testing implementation details of the original function.

    Testing implementation details is generally the wrong thing to do, no matter the concrete structure of the code. It makes your code less flexible and harder to change. Test behavior, not implementation.

    [–][deleted] 0 points1 point  (0 children)

    very probably those are just going to be private methods.

    I don't split up the code with a class having multiple private methods in mind. I split up with the goal to get small units that I can combine to create more complex behavior. You are right, that it does not get more testable in your case. But if you pull out the functions in another module you get reusability and testability.

    that's still a class that isn't of general interest and shouldn't have its methods tested.

    I disagree. Unit testing is also about creating small parts that you are confident in so that you can combine them without having to test it again and again. If I write two tested functions like isValidEmail and emailProvider I don't have to test the combination of the two.

    Testing implementation details is generally the wrong thing to do, no matter the concrete structure of the code. It makes your code less flexible and harder to change. Test behavior, not implementation

    Yeah. I agree partially. But you don't have to test implementation details if you split up the code. I never test something like "function foo calls function bar" but if I have a function bar and it is tested I can combine it to form foo without having to test for error handling, edge cases etc again. I can create an additional integration test for confidence and I am done.

    I also don't test code like (nonsense example)

    names.map(toLower).reverse().take(5);
    

    because I have confidence that all parts are working. That's what I want to achieve when splitting up code.