0
\$\begingroup\$

I have recently become interested in simulating logic circuits because of a electronics class at school. I have created a functioning simulator for logic gate circuits in Python. My implementation works, but I was wondering if it could be optimized or improved in any way? The project code is too big to fit in this question but is located at https://github.com/barrettkwise/circuitsim. Please let me know of any improvements that can be made.

print("thanks")
print("for")
print("your help!")
\$\endgroup\$
3
  • 1
    \$\begingroup\$ There is a reason that the question editor required code. Code needs to be embedded in the question for it to be on topic. \$\endgroup\$
    – Reinderien
    Commented Jun 1, 2023 at 3:14
  • \$\begingroup\$ If you are simulating logic gates, it makes no sense to use logic operators to simulate them. You can't simulate something with the thing being simulated. You should use predefined sets of inputs whose corresponding outputs should be true, and then generate the output by using membership checking or equality checking. Since all logic gates take at most two imports, you need at most four tuples for each logic gate, but all logic gates only output true for some inputs, you don't even need that many tuples. \$\endgroup\$ Commented Jun 1, 2023 at 3:50
  • \$\begingroup\$ @ΞένηΓήινος I don’t think you understand the purpose of the program. I am not simulating the gates, I’m simulating the interactions between the gates. \$\endgroup\$
    – turt1edman
    Commented Jun 1, 2023 at 15:16

1 Answer 1

1
\$\begingroup\$
Note: In the future, you should probably post a snippet of your code that you have the most doubts about or that is typical of your code, then provide a link to the rest. All good though!

Overall, your code is pretty good. It seems to work well, and it's well-formatted; I especially like your liberal use of type hints. I don't understand the reasoning behind the .line system, but that's beside the point.


In gateobject.py and simulator.py, you eval arbitrary user text. This is bad practice (though in this case, the fact that self.type is uppercased minimizes the potential security risk). If the user inputs an invalid gate, it will give an ugly error like this:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 1, in <module>
NameError: name 'BADGATE' is not defined

Instead, try using a dictionary (like operators = {'OR': OR, 'AND': AND...}); then, if self.type not in operators, you can handle the error from there (print an error message and exit, raise a custom error, etc.). This will make your code (a bit) cleaner, error handling/messages (somewhat) nicer, and your program (slightly) faster.

You could also make a helper function (like def eval_op(op: str, arg1: str, arg2: str) -> str) that handles that for you.


In several places, you import other files using extremely short names like s, ps or g. Generally, you want to avoid this, as it can lead to confusion and overlaps with variables in your code. The names aren't particularly long, so you could leave them as-is.


You often use enumerate, which is good! However, you specify enumerate(x 0), when the default is to start at 0 anyways. You can just leave it at enumerate(x).


In simulator.py:15-16, you count the variable p_level from 0 to len(self.circuit), incrementing it only at the end of each loop. This is better expressed with for p_level in range(0, len(self.circuit))


On simulator.py:18-21, you use the names curr_ids and curr_vals for ids and vlaues that are, according to your comment, not current. True, they are the ones you are currently working with, but you might want to rename them to avoid confusion.


On simulator.py:49-50, you do gate.out.value = eval(gate.type)(gate.in1.value, gate.in2.value), and again later without the second argument. You should probably separate this into a method on Gate, such as setOutput() or evalInputs().


On simulator.py:53, you eval(gate.type)(...) even though you know that gate.type == "NOT", and can just do NOT(...).


Throughout the whole program, you use "1" and "0" to denote true and false. If instead you replaced those with True and False, it will do two things: first, it would simplify your logicops.py functions (i.e. NOT(a: bool) -> bool: !a). Second, it would probably make your program run somewhat faster and/or use somewhat less space, since it's not dealing with strings, just booleans.


All in all, I would recommend you look for simpler ways to represent your data. You seem to be handy with the concepts, and effective at implementing them; you just need to simplify your code with builtins like for and bool. Please give me feedback on my feedback, and happy programming!

\$\endgroup\$
6
  • 3
    \$\begingroup\$ Hello at Code Review@SE. From How do I write a good answer?: Not all questions can or should be answered here. \$\endgroup\$
    – greybeard
    Commented Jun 1, 2023 at 5:56
  • \$\begingroup\$ Thank you for being so thorough in your assessment of my project, I really appreciate it. I’m aware that this question doesn’t exactly fit the criteria for the website, but I don’t know where else to go to get constructive criticism on a program this large. \$\endgroup\$
    – turt1edman
    Commented Jun 1, 2023 at 12:53
  • \$\begingroup\$ @greybeard I may have broken the "don't address every issue" rule (my bad), but the line you mentioned is talking about off-topic questions, and the sub-link says nothing about poorly-formatted questions. Constructive criticism, such as I led with, is the key to improving questions, not simply "get good, then we'll help out." Disregarding a question is only a valid response if the question is hopelessly flawed, which this is not. This is the second time today I have been referred to pages to improve something I have written here, read them thoroughly, and found nothing wrong with my post. \$\endgroup\$ Commented Jun 1, 2023 at 16:43
  • \$\begingroup\$ @turt1edman Your project does fit the criteria, you just need to be a bit more helpful in your question. Providing context, specific concerns, and code snippets is very helpful. See How do I ask a good question? for official guidelines on this topic. \$\endgroup\$ Commented Jun 1, 2023 at 16:46
  • \$\begingroup\$ (From /help/on-topic: If you can answer "yes" to all of the following questions, then your question is on-topic for Code Review: Is code included directly in my question? At the time of your answer (and comments up to this one), it is not. \$\endgroup\$
    – greybeard
    Commented Jun 1, 2023 at 17:35

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