4
\$\begingroup\$

I would like to create a small virtual PLC application, where the user may choose from a variety of logic gates and put them together to form a boolean program. I just have written the logic gates, which all work as intended. May you give me some tips in regarding programming style? My class Gate uses a static int variable named gateValuesForced. This variable shall be an indicator for the user that any of the Gates has used method Gate.force() and not has used Gate.unforce() in sucession. Did I've implemented this in the right way? Also in regard to my method Gate.finalize().

Package operatoren - Class Gate

package operatoren;

public final class Gate{
    private boolean value = false, force = false, forceValue = true;
    private static int gateValuesForced = 0;
    
    public final void setValue(boolean value) {
        this.value = value;
    }
    
    public final void flipValue() {
        if(value) value = false;
        else value = true;
    }
    
    public final void setForceValue(boolean forceValue) {
        this.forceValue = forceValue;
    }
    
    public final void flipForceValue() {
        if(forceValue) forceValue = false;
        else forceValue = true;
    }
    
    public final boolean force() {
        if(!force) {
            force = true;
            gateValuesForced++;
            return true;
        }
        else {
            return false;
        }
    }
    
    public final boolean unforce() {
        if(force) {
            force = false;
            gateValuesForced--;
            return true;
        }
        else {
            return false;
        }
    }
    
    public final boolean getValue() {
        if(force) return forceValue;
        else return value;
    }
    
    public final boolean getForceValue() {
        return forceValue;
    }
    
    public final boolean isForced() {
        return force;
    }
    
    public final static int gateValuesForced() {
        return gateValuesForced;
    }
    
    protected void finalize() {
        unforce();
    }
    
    public Gate() {
        
    }
}

Package operatoren - Class LogicGate

package operatoren;

import java.util.ArrayList;
import java.util.List;

public abstract class LogicGate {
    public List<Gate> input = new ArrayList<Gate>();
    public Gate output = new Gate();
    
    public abstract boolean calc();
    
    public LogicGate(int inputs) {
        for(int i = 0; i < inputs; i++) {
            input.add(new Gate());
        }
    }
}

Package operatoren - Class AND

package operatoren;

import java.util.NoSuchElementException;

public class AND extends LogicGate{
    public boolean calc() {
        if(this.input.size() > 1) {
            for(int i = 1; i < this.input.size(); i++) {
                if(this.input.get(i-1).getValue() && this.input.get(i).getValue()) {
                    this.output.setValue(true);
                }
                else {
                    this.output.setValue(false);
                    break;
                }
            }
            return this.output.getValue();
        }
        else {
            throw new NoSuchElementException("AND-Gate has less than 2 inputs.");
        }
    }
    
    public AND() {
        super(2);
    }
}

Package test - Class Test

package test;

import operatoren.*;

public class Test {
    public static void testLogicGate(LogicGate logicGate) {
        System.out.println(" ------- " + logicGate.toString() + " ------- ");
        boolean intToBoolean = false;
        for(int i = 0; i < (1 << logicGate.input.size()); i++) {
            for(int j = 0; j < logicGate.input.size(); j++) {
                if((i & (1 << j)) > 0) {
                    intToBoolean = true;
                }
                else {
                    intToBoolean = false;
                }
                logicGate.input.get(j).setValue(intToBoolean);
                System.out.println("logicGate.input.get(" + j + ").getValue() = " + logicGate.input.get(j).getValue());
            }
            logicGate.calc();
            System.out.println("logicGate.output.getValue() = " + logicGate.output.getValue() + "\n");
        }
        if(Gate.gateValuesForced() != 0) {
            System.out.println("Gate.gateValuesForced() = " + Gate.gateValuesForced());
        }
    }
    
    public static void main(String[] args) {
        AND testAND = new AND();
        
        /* uncomment one or more of following lines for testing */
        //testAND.input.add(new Gate());
        //testAND.input.get(0).setForceValue(false);
        //testAND.input.get(0).force();
        
        testLogicGate(testAND);
    }
}

You may uncomment one or more of those comments to view another test situation.

\$\endgroup\$
4
\$\begingroup\$

Few suggestions:

  • The class Gate is final, therefore all its methods are implicitly final, no need to specify the modifier on each method. Docs.
  • The name Gate is a bit confusing because that class represents an input or output.
  • One variable per declaration: private boolean value = false, force = false, forceValue = true; is not considered good practice.
  • You can refer to Google Java Style Guide for additional style improvements
  • The method Gate#flipValue:
    public final void flipValue() {
        if(value) value = false;
        else value = true;
    }
    
    Can be simplified to:
    public final void flipValue() {
        value = !value;
    }
    
  • Regarding the method AND#calc:
    public boolean calc() {
        if(this.input.size() > 1) {
            for(int i = 1; i < this.input.size(); i++) {
                if(this.input.get(i-1).getValue() && this.input.get(i).getValue()) {
                    this.output.setValue(true);
                }
                else {
                    this.output.setValue(false);
                    break;
                }
            }
            return this.output.getValue();
        }
        else {
            throw new NoSuchElementException("AND-Gate has less than 2 inputs.");
        }
    }  
    
    First, the output of this method is not used in other parts of your code so it can be void. Second, it's enough to find one input==false to set the output to false, therefore the logic can be simplified. Third, checking the input size in this method is too late, it should be done in the constructor.
  • The constructor of the class AND accepts an argument but ignores it.
  • It's better to expose the instance variables of LogicGate externally only via methods, but at the same time let the subclass access them. To do that, change the modifier from public to protected.

Testing

public class Test {
    public static void testLogicGate(LogicGate logicGate) {
        // ...
    }
    
    public static void main(String[] args) {
        AND testAND = new AND();
        
        /* uncomment one or more of following lines for testing */
        //...
        //...
        //...
        testLogicGate(testAND);
    }
}

Every time you run this test, you need to check the results by looking at the output. Add 5 or 10 more tests and this approach becomes unfeasible. Consider to use a more complete testing tool such as JUnit that checks the results automatically. For example:

public class ANDTest {
    @Test
    public void testCalc() {
        AND and = new AND(2);
        and.setInput(0, true);
        and.setInput(1, true);
        
        and.calc();

        assertTrue(and.getOutput());
    }
    // other tests..
}

Design

Keeping track of how many times a gate value is forced can be done by a class that uses or manages Gate, not Gate itself (with a static variable). If you move that logic out of Gate, then it becomes just a wrapper for a Boolean. In that case, LogicGate can directly have a list of Boolean as inputs.

In general, it's a lot of code for a multi-input AND, but I guess the interesting part will be to combine different operators together.

Note that a multi-input gate can be made by joining multiple (two-input) gates of the same type.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you very much for your good feedback. I'll implement your solid ideas :-) I've one question, regarding the modifiers. When my class is already public, do my methods of that class also become per default public? Does the default modifier always become the same like the "super" modifier? \$\endgroup\$
    – paladin
    Oct 23 '20 at 0:47
  • 1
    \$\begingroup\$ @paladin Glad I could help. First question: no, if the class is public and the methods are private, they won't be visible externally. Second question: there is no such ''super" modifier, maybe you mean the class modifier? If a class is public and the methods have default modifier, they won't be visible in other packages. Check more info here or the official doc. \$\endgroup\$
    – Marc
    Oct 23 '20 at 6:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.