Is it better for decision-making logic to live inside the class that acts on the decision or in a separate class?

StackOverflow https://stackoverflow.com/questions/21483194

  •  05-10-2022
  •  | 
  •  

Question

Take a simplified situation: I've got a class whose job it is to create certain files at a given path (let's call it FileCreator). The files only need to be created, however, if they don't already exist at said path.

Is it better for FileCreator to check whether the files exist at the path and then create them if they don't, or do I create a second class that's responsible for checking (FileChecker) and let FileCreator purely do the creating (without any regard for whether they actually exist or not)?

Situation 1: Decision logic sits in class dependent on decision

class FileCreator
  def initialize(path)
    @path = path
  end

  def create_files
    unless files_exists?(path)
      #create files at path
    end
  end

  def files_exists?(path)
     #check if files exist at path
  end
end

file_creator = FileCreator.new('/foo')
file_creator.create_files

Situation 2: Decision logic sits in own class

class FileChecker
  def initialize(path)
    @path = path
  end

  def files_exists?(path)
     #check if files exist at path
  end
end

class FileCreator
  def initialize(path)
    @path = path
  end

  def create_files
    #create files at path
  end
end

file_checker = FileChecker.new('/foo')
file_creator = FileCreator.new('/foo')
file_creator.create_files unless file_checker.files_exists?

The first scenario is more convenient, but I imagine that the second scenario is more flexible (in the sense that I now know exactly which class is responsible for exactly what, so I can easily juggle them around).

I'm quite new to programming, so any external references to thoughts or patterns on this particular question will be appreciated (also, not sure that this question is tagged correctly).

Was it helpful?

Solution

I would keep as simple as possible (but no simpler ;-)

In your deliberately simplified example I would have a single class, the Check functionality is closely related to the Create functionality so there a degree of cohesiveness, so we can argue that this code belongs together.

I'd be motivated to refactor the code into separate classes under some conditions. Before explaining some such conditions, note the thought process: I know I can refactor should I need to, and indeed I can probably do so without perturbing any users of the Create functionality; the design has some intrinsic flexibility, we're just thinking about rearranging the internals of the Create implementation.

Some possible reasons to refactors into two classes:

  1. The checks we need to do might become more complex - eg. check for permissions, for space ... - we might want run-time selectivity of the checks, feels like a hierarchy of Check classes might be emerging, time to refactor.
  2. We need to reuse the check in another context, in some other class.
  3. The check code becomes complex in its own right, we need to refactor it's code so we can understand it, we grow lots of implementation functions specifically for Check. At this point our Create class becomes cluttered, time to refactor.
  4. As complexity increases we see the need for separate testing of Check, so it has become a first-class Class - needs documenting and controlling in its own right.

OTHER TIPS

I would lean more toward putting both functionalities in the same class. OOP is about combining data with behavior in a single object. In this case, your data is your path and file name, and you have two behaviors associated with that data: checking for existence, and creation.

With that in mind, I think it would be more OOP-like to have this object be called File instead of File + some verb. After all, OOP is about modeling an object, and the verbs of that object are its methods.

My implementation might look something like:

public class File
{
    private readonly string path;

    public File(string path)
    {
        this.path = path;
    }

    public void Create()
    {
        if (!DoesFileExist())
        {
            // create file.
        }
    }

    private bool DoesFileExist()
    {
        // check if file exists.
    }
}

The answer is that it probably doesn't matter; however, single responsibility classes are generally more reusable, especially if you have multiple inheritance. Unless there's a specific reason to tie them together, you might as well have them separate.

I would probably have written this example as one class, if there were no other consideration. However, once it becomes something to deliberate about, you might as well just have them separate.

In a simple scenario like that probably the best option is the first, remember that the final objective of software design is to produce systems easy to understand/modify/extend, and the first class is really easy to understand.

The answer from djna its really very good, refactor when there is a clear benefit. When the day to refactor came, considerer another option, you have two responsibilities:

  • Create a file
  • Create only when some criteria is meet

You can have something like this:

class FileCreator

  def initialize(path, fileCreationCriteria)
    @path = path
    @fileCreationCriteria = fileCreationCriteria
  end

  def create_files
    unless fileCreationCriteria.meetCriteria?(path)
       #create files at path
    end
  end

end

Then you can have different criteria, for example that the file already exits, another example can be that the is a space limit o whatever. When you create your object you do:

file_creator = FileCreator.new('path', CreateIfNotExists.new)

file_creator = FileCreator.new('path', CreateAllwaysAndOverride.new)

file_creator = FileCreator.new('path', CreateUntilSpaceLimitIsReached.new('1GB'))

Probably having one default criteria can be a good idea, think in made things easy for the caller of your class/api.

Obviously this is more than over-engineering for a problem like the initial you present, its only as an example of SRP and Open-close (now your file creator its close to modification and open for extension and have only one responsibility).

Remember that the patterns and principles are only guides, always depending on the context and on the good judgement of the designer.

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