Question

We currently have 3 devs with, some, conflicting styles and I'm looking for a way to bring peace to the kingdom...

The Coders:

Foo 1: Likes to use Func's & Action's inside public methods. He uses actions to alias off lengthy method calls and Func's to perform simple tasks that can be expressed in 1 or 2 lines and will be used frequently through out the code

Pros: The main body of his code is succinct and very readable, often with only one or 2 public methods per class and rarely any private methods.

Cons: The start of methods contain blocks of lambda rich code that other developers don't enjoy reading; and, on occasion, can contain higher order functions that other dev's REALLY don't like reading.


Foo 2: Likes to create a private method for (almost) everything the public method will have to do .

Pros: Public methods remain small and readable (to all developers).

Cons: Private methods are numerous. With private methods that call into other private methods, that call into... etc, etc. Making code hard to navigate.


Foo 3: Likes to create a public class with a, single, public method for every, non-trivial, task that needs performing, then dependency inject them into other objects.

Pros: Easily testable, easy to understand (one object, one responsibility).

Cons: project gets littered by classes, opening multiple class files to understand what code does makes navigation awkward.


It would be great to take the best of all these techniques...

Foo-1 Has really nice, readable (almost dsl-like) code... for the most part, except for all the Action and Func lambda shenanigans bulked together at the start of a method.

Foo-3 Has highly testable and extensible code that just feels a bit "belt-&-braces" for some solutions and has some code-navigation niggles (constantly hitting F12 in VS and opening 5 other .cs files to find out what a single method does).

And Foo-2... Well I'm not sure I like anything about the one-huge .cs file with 2 public methods and 12 private ones, except for the fact it's easier for juniors to dig into.

I admit I grossly over-simplified the explanations of those coding styles; but if any one knows of any patterns, practices or diplomatic-manoeuvres that can help unite our three developers (without just telling any of them to just "stop it!") that would be great.

From a feasibility standpoint :

  • Foo-1's style meets with the most resistance due to some developers finding lambda and/or Func's hard to read.
  • Foo-2's style meets with a less resistance as it's just so easy to fall into.
  • Foo-3's style requires the most forward thinking and is difficult to enforce when time is short.

Any ideas on some coding styles or conventions that can make this work?

Was it helpful?

Solution

I'd say that before you introduce new rules and conventions, that you should set aside some team time and initiate a respectful and open discussion within your development team about the current codebase. Get your developers to discuss coding styles and the things that they both like and dislike about the codebase and its mixture of coding styles.

In short, get things out in the open; this is more healthy than letting feelings fester, and even if you introduce new rules to fix the code issues, the politics will likely remain.

Let the team feel that they are being treated as sensible adults and that they have some input and influence upon the coding conventions that you introduce.

It's always worth trying a self-policing approach first, and you may not need to enforce anything, if the discussion goes well.

Most important of all: Try to ensure that everyone listens (including yourself).

OTHER TIPS

It's not at all clear why you dislike the private methods for Foo-2.

You complain about a "huge" .cs file - but why would it be significantly larger than Foo-1's style? The same amount of code is there, it's just that the actions are split into methods rather than expressed as lambdas.

Foo-2 seems the best option to me, to be honest. Keep your public API to only what you want to expose it, and then implement it in the simplest way. While lambdas are definitely appropriate in some situations, Foo-1's style sounds like it takes it to the extreme - beyond where it's really sensible.

In particular, consider if your two public methods have some common subtask. Foo-1's approach would end up duplicating that code - Foo-2's approach would put the common code in a private method called from both... which seems like a sensible approach to me.

Likewise, you talk about private methods calling private methods... surely the equivalent Foo-1 code would be lambda expressions calling lambda expressions, which is hardly any better!

Additionally, if you're happy with white-box unit testing, Foo-2's approach can make it easier to test "chunky" public API implementations by testing the "smaller" bits of the implementation individually - admittedly forcing you to use internal visibility instead of private for the methods, or using reflection.

Foo-3 sounds like a different approach entirely, in that it's changing the public API rather than the implementation. That's more about design than coding style, and should be considered separately.

For my 2 cents, they all have their place but no one style should be used exclusively, especially 1 and 3. Style 1 has code that is difficult to understand, and style 3 results in an object model that is difficult to work out.

Get together and try and work it out. This is surprisingly difficult to do and is often more about compromise to get consistency than doing what is 'right'. The compromisers are the unsung heroes here. :)

Foo 1

I must admit I like to use lambda and funcs occasionally to program in a "co-routine" style. However, this is rare and only used when alternatives would not be as neat, or express the design intent as clearly.

I really don't think that this style of coding scans well as a general programming style (at least in C#) as many other programmers have to play compiler to work out what is going on - not good. However it certainly does have its place.

Foo 2

This sounds like a reasonable coding style. The fact that there are dependencies between private methods is just DRY working on the small scale (if it is executed well).

Foo 3

Using DI does not mandate using this style i.e. single public method per class. This style at its worst is symptomatic of forgetting that in OO we are actually trying to model 'things'. Gluing together a huge bunch of methods with no discernible object model is not great, discoverability sucks. Creating good object models is hard though.

It is really very tricky to convenience developers to follow strict guidelines, especially when a new language features come along, especially Foo-1 developer want to use lambda.

It looks like Foo-1 and Foo-2 have some similarities to use more functional programming.

As in your team, the primary development language is C#, which is mainly object oriented, so you must try to bring more object oriented approach towards development and try to persuade them to use classes / objects for reusable code. One of the technique is peer review would also help among your team members, it is not required for every line of code, but couple of starting sessions may help, and you should manage / start those reviews with them first and then let them do.

I am sure that you will get alot of differing opinions on this, but my feeling is that the internal implementation of a class is hidden and really doesn't make that much of a difference to the consumer of the class. Therefore, Foo1 and Foo2 are basically the same and the only difference is Foo3. I am also not a fan of having multiple objects all over the place. If I had to lean towards Foo1 or Foo2, I would favour Foo2 because it would be easier to refactor and move code into subclasses.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top