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.
1 Answer 1
Few suggestions:
- The class
Gate
isfinal
, therefore all its methods are implicitlyfinal
, 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
:
Can be simplified to:public final void flipValue() { if(value) value = false; else value = true; }
public final void flipValue() { value = !value; }
- Regarding the method
AND#calc
:
First, the output of this method is not used in other parts of your code so it can bepublic 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."); } }
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 frompublic
toprotected
.
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.
-
\$\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– paladin2020年10月23日 00:47:34 +00:00Commented Oct 23, 2020 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– Marc2020年10月23日 06:25:21 +00:00Commented Oct 23, 2020 at 6:25