0

I have several areas in a program where I am doing the following check on the same two booleans, but each spot has different text being written to a file via Stream Writer based on the value of the booleans.

Example

if (bool1 && bool2)
{
  StreamWriter.WriteLine("Stuff to the file");
  StreamWriter.WriteLine("Stuff 2 to the file");
  StreamWriter.WriteLine("Stuff 3 to the file");
}
else if (bool1) 
{
  StreamWriter.WriteLine("Stuff 4 to the file");
  StreamWriter.WriteLine("Stuff 5 to the file");
  StreamWriter.WriteLine("Stuff 6 to the file");
} 
else if (bool2) 
{
  StreamWriter.WriteLine("Stuff 7 to the file");
  StreamWriter.WriteLine("Stuff 8 to the file");
  StreamWriter.WriteLine("Stuff 9 to the file");
}

The thought process is, if both Booleans are true, write certain text to the file. Else if only bool1 is true, then write this bool1 specific text. Else if only bool2 is true, then write this bool2 specific text.

There are at least 3 places in my program where I'm doing these comparisons, with the WriteLine text based on the Boolean values being different in each place.

Is there a better way to do this kind of comparison logic? Such that I wouldn't need to be checking the same booleans over and over to write the various texts to the file.

I've considered writing a method that takes in 3 values representing what should be written to the file based on the Boolean comparisons. I am struggling however to understand how I would pass WriteLine calls as parameters to a method.

Godwin
  • 11
  • 4
  • No, there's no "standard." Try this: [Avoid Else, Return Early](https://blog.timoxley.com/post/47041269194/avoid-else-return-early). – Robert Harvey Jul 09 '19 at 19:43
  • See also https://softwareengineering.stackexchange.com/q/18454 – Robert Harvey Jul 09 '19 at 19:47
  • @RobertHarvey Thanks, in my specific use case, I don't want to return from the method as much as I want to write different text to the file based on the boolean value. I've updated my question to try and add more details. – Godwin Jul 09 '19 at 20:10
  • 3
    You can covert this to a switch. You have three **cases**: Both, JustOne and JustTwo. Define an enum for these cases and determine the case upfront. Then switch on it. – Martin Maat Jul 09 '19 at 20:14
  • You could do it via a method or an object as you intended (you can pass in an `Action` (lambda) for each case), but it would be of limited benefit - you'd pretty much get the same construct, only in a different syntax. You would likely capture some of the decision-making logic, and communicate to your future self and other developers that 3 actions need to be provided - and maybe that's all you need. But if the problem is that you have cascading changes and coupled code (you're always making changes in several places/files), this approach doesn't solve that. – Filip Milovanović Jul 09 '19 at 22:00
  • "*I am struggling however to understand how I would pass WriteLine calls as parameters to a method*". That bit is easy: the parameter is just `Action` and you'd supply a simple lambda like `s => StreamWriter.WriteLine(s)` to that parameter when calling the method. – David Arno Jul 10 '19 at 06:31
  • @FilipMilovanović I understand what you're saying. I'm going to attempt a method implementation but as you said, it doesn't necessarily solve the issue. If anything this has given me more to think about. Thanks – Godwin Jul 10 '19 at 14:21
  • If the contents you're writing to the file are different in all three cases, then I don't think you can compress this much more than you are - a function taking lambdas would be effectively the same size and harder to read. If the comparison itself is heavy, do it once and store the result. – Errorsatz Jul 12 '19 at 21:02

3 Answers3

2

Another way to solve it using the Design Patterns Strategy and Factory Method:

An interface defines the strategy, in this case, it say that the strategy is write something, without details about what.

interface IStrategy
{
    void Write();
}

For each variation of the strategy you implement the IStrategy:

class StrategyA : IStrategy
{
    //....
}
class StrategyB : IStrategy
{
    //....
}
class StrategyC : IStrategy
{
    //....
}

A factory method help you change the behavior of your application in runtime defining a strategy from parameters:

public static IStrategy Create(string type)
    {
        switch (type)
        {
            case "A":
                return new StrategyA();
            case "B":
                return new StrategyB();
            case "C":
                return new StrategyC();
            default:
                throw new NotImplementedException();
        }
    }

Usage is simplified to:

public void Write()
{
    using (var sw = new StreamWriter())
    {
        var strategy = StrategyFactory.Create("A");

        strategy.Write(sw);
    }
}

Notice you can pass the value "A" dynamically, also getting it from database:

And so the result is:

interface IStrategy
{
    void Write(StreamWriter streamWriter);
}

class StrategyA : IStrategy
{
    public void Write(StreamWriter streamWriter)
    {
        streamWriter.WriteLine("Stuff to the file");
        streamWriter.WriteLine("Stuff 2 to the file");
        streamWriter.WriteLine("Stuff 3 to the file");
    }
}

class StrategyB : IStrategy
{
    public void Write(StreamWriter streamWriter)
    {
        streamWriter.WriteLine("Stuff 4 to the file");
        streamWriter.WriteLine("Stuff 5 to the file");
        streamWriter.WriteLine("Stuff 6 to the file");
    }
}

class StrategyC : IStrategy
{
    public void Write(StreamWriter streamWriter)
    {
        streamWriter.WriteLine("Stuff 7 to the file");
        streamWriter.WriteLine("Stuff 8 to the file");
        streamWriter.WriteLine("Stuff 9 to the file");
    }
}

class StrategyFactory
{
    public static IStrategy Create(string type)
    {
        switch (type)
        {
            case "A":
                return new StrategyA();
            case "B":
                return new StrategyB();
            case "C":
                return new StrategyC();
            default:
                throw new NotImplementedException();
        }
    }
}

class Program
{
    public void Write()
    {
        using (var sw = new StreamWriter())
        {
            var strategy = StrategyFactory.Create("A");

            strategy.Write(sw);
        }
    }
}

This solution depends on the size of your system complexity. Otherwise, it may not be worth it. But still, it is a useful alternative.

phduarte
  • 286
  • 1
  • 5
  • While I think the OP's solution will involve passing the two booleans to the strategy factory, I think this is the best solution. It solves the issue of code reuse and performing this check in multiple places. – Greg Burghardt Jul 30 '19 at 12:09
  • Yes, I agree. Although there's a lot alternatives, this is one. Passing booleans can also be interesting, I still suggest Enums if strings is not a good choice. – phduarte Jul 30 '19 at 12:23
  • This is probably the best solution. For my situation however, what's being written to the file is so trivial that implementing this design pattern seems like overkill. Shortly after posting the question I went with @FilipMilovanović suggestion of creating a method that takes 3 actions, and then writing a switch the performs each action as appropriate. I'm going to look into an implementation of this pattern as I view it as an improvement. Thanks! – Godwin Jul 30 '19 at 19:14
  • 1
    +1 - but the implementation would be much nicer if `Strategy` was an interface and `sw` was a method parameter for `Write` instead of a constructor parameter. Constructor parameters are for implementation-specific dependencies - the streamwriter is part of the contract for `Write` (the consumer expects the passed stream to be written to when write is called). – Ant P Jul 31 '19 at 08:17
  • Using the interface is a good idea. This is another way to solve this problem and the implementation with the interface is very similar. :) – phduarte Jul 31 '19 at 12:01
  • @phduarte I agree swapping an Interface for the Abstract class is solid. I saw that being used here. https://dzone.com/articles/design-patterns-the-strategy-and-factory-patterns – Godwin Jul 31 '19 at 15:50
  • 1
    I edited the answer to give an example of the same solution however using Strategy with Interface. Thank you all for your suggestions. – phduarte Jul 31 '19 at 16:51
0

Use a procedure to do the branching, either:

WriteLines(IEnumerable<string> linesBoth, IEnumerable<string> linesOnlyA, IEnumerable<string> linesOnlyB)
{
    // ...
}

or:

WithStream(Action<StreamWriter> onBoth, Action<StreamWriter> onA, Action<StreamWriter> onB)
{
     // ...
}
Kasper van den Berg
  • 2,636
  • 1
  • 16
  • 32
0

Since there's clear coupling among the classes within your project, for this problem, you could and probably should utilise the template method pattern. It introduces the same coupling but makes sure it's present only once. And the implementation can then focus only on the parts which are different, rather than also taking care of the execution of methods based on conditions.

You create a class acting as a template, indicating how and in which order methods are to be called:

public abstract class MyStreamWriterTemplate
{

    public final write(...)
    {
        if (bool1 && bool2)
        {
            this.writeWhenBothTrue();
        }
        else if (bool1)
        {
            this.writeWhenFirstTrue();
        }
        else if (bool2)
        {
            this.writeWhenSecondTrue();
        }
    }

    protected abstract writeWhenBothTrue();

    protected abstract writeWhenFirstTrue();

    protected abstract writeWhenSecondTrue();
}

and a concrete implementation then inherits this template class and provides custom functionality:

public class WriterA extends MyStreamWriterTemplate
{
    private final StreamWriter streamWriter;

    public WriterA(StreamWriter streamWriter)
    {
        super();

        this.streamWriter = streamWriter;
    }

    protected writeWhenBothTrue()
    {
        streamWriter.WriteLine("Both true from A");
    }

    protected writeWhenFirstTrue()
    {
        streamWriter.WriteLine("First true from A");
    }

    protected writeWhenSecondTrue()
    {
        streamWriter.WriteLine("Second true from A");
    }
}

If you later find out that for certain cases, you only really need to write e.g. when only the second attribute is true, you could change the template to not have abstract methods but to rather provide default implementation of doing nothing.

That way you won't force the actual implementing classes to implement the abstract methods just to provide no behaviour.

Andy
  • 10,238
  • 4
  • 25
  • 50