Creating and sending a Fixed-Width file

Question

This function will run every x seconds.

PAN: The name we are calling our fixed width file we are transmitting.

Private Sub Start()
 ' Get the data from the API so we can get ready to make the PAN File
 Dim Baker As New PAN.Maker
 Baker.SetIngredients(GetAPIData)
 ' Make the PAN file
 Baker.Make()
 ' Retrieve the PAN file from the baker
 Dim PAN As PANv1 = Baker.GetPAN()
 ' Transmit the PAN file, via FTP
 Dim PANSender As New PAN.Transmitter(GetFTPDetails)
 PANSender.Send(PAN)
End Sub

• I create the Objects Maker and Transmitter in a PAN namespace to group the functionality. Is this wise?

• I decide to use a method to set the data it needs before it can create the PAN file, rather than using a constructor. What is the preferred method in this case?

• Am I using too many Objects to handle these tasks, when a more procedural structure would do?

• Is the naming of the Baker and Transmitter confusing because they differ from the Objects name?

• Are these straightforward and easily understandable OOP practices?

• Simon Forsberg: What is a PAN file?
• Harry Mumford-Turner: @SimonAndréForsberg The name we are calling our fixed width file we want to send.
• Mathieu Guindon: As you can see with the answer I managed to give you, reviewing code isn't something that's done off a minimalistic sample of what the code seems to be doing - we need meat, context. I hope your future CR posts give us more code to look at! =)

Answer 1

This function will run every x seconds.

That's great. But without more context, there is no way we can identify any potential issues with the code you've provided.

Private Sub Start()

Depending on what the containing class is called, that method's name isn't very clear. And why is it Private? Who's calling it? A method like Start() sounds like something that belongs on the public interface; I'm confused.

If I were maintaining this code, I would remove every single comment you have there. Why? Because every single one of these comments is restating what the code does, and good comments should say why the code does what it's doing.

---

I create the Objects Maker and Transmitter in a PAN namespace to group the functionality. Is this wise?

It looks like you have very specialized objects, and that is very good thing. Whether or not they deserve their own namespace isn't something that can be commented on, because we have no idea what other things you have here: for all I know every type involved in what you've shown could be in the same namespace, it depends on the scope of your application: if it's an ERP system with 4,327 other functionalities, then yes, definitely put that in its own namespace. If it's a specialized application that only deals with "PAN" files, I don't know.

I decide to use a method to set the data it needs before it can create the PAN file, rather than using a constructor. What is the preferred method in this case?

I'm guessing you're referring to this line:

Baker.SetIngredients(GetAPIData)

What's GetAPIData? It looks like it could be a member call (a property getter?), or a field - in either case, the name is a bad one: GetAPIData would be a name for a method. Without seeing its declaration, it's very hard to tell what's going on here - heck, it could even be a delegate (in which case the name isn't too bad after all).

The point is, if it's just data, then there's no reason to not pass it through the constructor. If it's work to do, then you've done the right thing: constructors shouldn't do work.

Am I using too many Objects to handle these tasks, when a more procedural structure would do?

That's primarily opinion-based, but VB.NET, like C#, has an object-oriented paradigm; procedural "script-like" code would work, but if you want SOLID code, it just won't do.

Again, we have no idea what your objects are actually doing, so it's not possible to give you an answer here, but if your objects essentially are made of one or two one-liner methods, OOP is probably overkill.

Is the naming of the Baker and Transmitter confusing because they differ from the Objects name?

I'm confused here. Baker is an object name:

Dim Baker As New PAN.Maker

And Transmitter is a type name:

Dim PANSender As New PAN.Transmitter(GetFTPDetails)

PAN looks like it's a class name, and Transmitter would be a nested type - I don't like this. If PAN is a namespace, it's a bad name and Transmitter shouldn't be fully-qualified like this.

Dim PAN As PANv1 = Baker.GetPAN()

The local variable PAN confuses things even further - is it the same PAN as seen in PAN.Transmitter? That makes Transmitter a shared method, and that's starting to smell.

• Harry Mumford-Turner: Thank you for the response, the last quarter of the post I found really useful
• Mathieu Guindon: @Mumf you're welcome! I would strongly recommend posting another question, with as much context (code) as possible - then you'd get actual reviews.. this answer is only a shadow of what this site can do for you! :)
• Harry Mumford-Turner: The problem I had was that only half of that code was implemented! I needed initial layout advice and things would link together, I shall implement more and return! Thank you for your help :)