rna-transcription: rename "ofDna" with "transcribe"

[jtigger]

Had a conversation with @chuckwondo on the Java track about the name of the method in rna-transcription; through it, it became clear that ofDna() lacks a certain clarity that a verb like transcribe() would include more punch.

Any objections to renaming that method?

[FridaTveit]

Sounds like a good idea :)

[chuckwondo]

What about also making the method static and updating the test suite to match? It seems odd to me to be forced to create a stateless instance just so that the method can be invoked.

[FridaTveit]

We've made it a policy to prefer instance methods over static methods. See issue #177 for discussion of why :)

[jtigger]

To be clear, @chuckwondo, the things that we're finding that "need" a policy are exactly those stylistic grey areas. As I think clear in the policy issue itself, there's good reasons for either approach.

There's definitely "inertia" behind the instance-based approach, but there's always room for discussion. Always.

[chuckwondo]

Thanks @jtigger. I'm glad to see that there are documented policies, and I'm glad there's room for discussion.

For this particular policy, I'm not sure I agree with always using instance methods instead of static methods is the most effective stance. There are good reasons for using one or the other in various scenarios.

At the very least, if you want to stick with instance methods, then I suggest designing the usage to better align with that choice. In particular, using an instance method is generally used in conjunction with a stateful object. Therefore, if you're going to invoke an instance method, it would appear to be better aligned with passing state to a constructor, not to the method.

Passing everything required for a computation to the method performing that computation implies there's no need to have an instance, because there's no need for it to hold state information.

Therefore, I propose that for instance methods, we should pass state information to the constructor and pass nothing to the method (unless there's a need to do so beyond the state information held by the instance). For static methods, obviously pass all necessary inputs directly to the method.

What I am not fond of in this case with rna-transcription (and all of the other problems that follow suit) is the creation of an instance that holds no state, only to invoke a method on that instance to which all necessary input is passed. Either make the method static, or pass the state to the constructor (and pass nothing to the method).

For the less experienced programmers, mixing things up like this has the potential to instill arguably poor coding practices.

[FridaTveit]

I'm not sure always using instance methods is instilling poor coding practices. Having static methods can make it harder to write tests. I used to always make things static if I could, and I've had to unlearn that as it's caused me problems when trying to test things. So like @jtigger said, I think there are good reasons for either approach :)

I'm also not sure we should be passing arguments to the constructor for rna-transcription. Passing arguments to the constructor and not the method makes sense in some cases, when the arguments really are state and therefore have more to do with the object than the method. Sometimes the nature of the object means that by passing the arguments to the constructor instead of the method, we then need to construct a new object for every time we call the method. In that case I think we should be passing the arguments to the method, not the constructor.

[chuckwondo]

I agree that there are good reasons for each approach, which is why I'm saying that enforcing the same approach across all problems is a questionable policy. Such a policy is then specifically excluding the other approach even for cases where the excluded approach would arguably be the better approach.

Regarding having to construct a new object when the constructor takes an argument rather than having the method take the argument is not what I would consider a valid reason for switching things around (i.e., passing nothing to the constructor and passing the argument to the method).

I believe the determining factor is whether you intend to create an immutable object versus a "utility" method, which is my basis for this discussion.

In the case of creating a "utility" method, common practice is to define a static method, not to create an instance and then call an instance method.

Regarding writing tests for static methods, I don't see any difference in difficulty. Can you provide an example of where writing a test would present difficulty within the context of the relatively simple problems here? (Of course, for much more complicated systems where we might be dealing with interfaces, dependency injection, and the like, I agree that using static methods might not be the right approach.)

In the case of an immutable object, then that's where it makes sense to pass state to the constructor, then call an instance method with no arguments (unless arguments beyond the immutable state are required for the computation).

The way rna-transcription is currently written, it is a mix of the above. That is, there is no state (like a static utility method), but it is necessary to create an instance.

I'm suggesting either make it a proper utility method by making it a static method, or make it immutable by passing the argument to the constructor, then invoking the instance method with no arguments.

The mashup of these 2 approaches with rna-transcription (and other similarly structured solutions) is what I'm saying is arguably poor practice.

In other words, it doesn't make sense (in the simple problems here) to create a stateless instance, then invoke an instance method with the only information required for the computation. Either make it stateful (passing the argument to the constructor), or make it static. Don't make it stateless and non-static (unless, again, you are creating a much more sophisticated application that might warrant it).

In the end, it's not really a big deal. I'm just pointing this out as a minor code smell IMHO.

[FridaTveit]

I don't mean that it'll make it more difficult to test here, I mean in general when making something more complicated. As far as I'm aware part of our reasoning behind making all methods instance methods is to encourage good practices, not because the methods necessaily need to be for these simple exercises. For example, like I said I've had to unlearn making everything static. That was because when being taught programming the examples I saw were making methods static whenever they could. So arguably, by encouraging people who are new to programming to use static methods they can encounter problems when having to contribute to a more complicated codebase.

Also, you could argue that often when you initially create a method, the code around it might be simple enough that it doesn't cause any problems for the method to be static. But then later on the codebase around it grows and it becomes a problem. So not making methods static could be a safe approach. I know that that won't happen here, but again, encouraging good practices :)

But I don't really mind either way. I don't think having them all be instance methods is bad practice. But I also don't think it would be the end of the world if we used static ones. I don't think passing in arguments to the constructor for rna-transcription is the best idea, but I could be persuaded if other people think it is :)

[stkent]

Was this closed in #521?

[FridaTveit]

The original problem this issue was discussing was closed in #521. The discussion about whether we should be using static methods or not wasn't but maybe we should keep discussing that in a different issue? Or are you happy with things as they are @chuckwondo?

[FridaTveit]

I'll close this as we've implemented the changes in the original question. Feel free to open a new issue about static methods if you want @chuckwondo :)