Skip to main content
29 votes
Accepted

Python - Tkinter - periodic table of chemical elements

I hope you enjoy this bit of code. I did! Your symbols, keywords and values should have capitalised variable names since they're global constants. However, life would be easier if your symbols were ...
Reinderien's user avatar
  • 71.1k
27 votes

Python - Tkinter - periodic table of chemical elements

Don't optimize the wrong thing You have an excellent review already, so I'll comment on a narrow topic: cramped code layout. For example: ...
FMc's user avatar
  • 13.1k
15 votes
Accepted

Numerical integration in C++: Newton-Cotes formulas

I wonder whether the #include "NewtonCotesFormulas.cpp" at the end of the NewtonCotesFormulas.h header file could be somehow avoided. I used it because without I'd get linking errors. Yea, ...
JDługosz's user avatar
  • 11.7k
13 votes
Accepted

Creating HTTP proxies for services

Create a strategy to find the mapped implementation of the interface provided: ...
Nkosi's user avatar
  • 3,296
12 votes

Numerical integration in C++: Newton-Cotes formulas

Use an enum to give names to choices Consider using an enum, or even better an enum class, ...
G. Sliepen's user avatar
  • 69.3k
8 votes

Chess move validator

Use Standards Use standard notations and representations whenever possible: see Algebraic Notation for example. Avoid Global Variables Board.CHESS_BOARD is a ...
abuzittin gillifirca's user avatar
7 votes

Chess move validator

Thank you for releasing your code! You have some clever solutions and I would like to add few more points to the previous answers. Naming Piece The class Pawn ...
Roman's user avatar
  • 2,923
7 votes

Factory pattern using enum

Using enums in Python The posted code doesn't take advantage of most of the benefits provides by enums in Python. The typical use case of enums is to make decisions depending on a given enum member. ...
janos's user avatar
  • 113k
6 votes
Accepted

Shape area calculation

I think your names could be simplified. For instance, the ShapeType enum could be: ...
Rick Davin's user avatar
  • 6,752
5 votes
Accepted

Object-oriented calorie counter

I like your code. It is easy to understand and pretty straight-forward. Regarding your question of the measuring units: yes, every variable should have them since you are not operating exclusively on ...
Roland Illig's user avatar
  • 21.9k
5 votes
Accepted

Abstract Factory Pattern Implementation

This code looks fine, it is indeed an implementation of a factory pattern. However, why would both Audio and Video be in the same factory? Apart from having the same responsibility to export something,...
IEatBagels's user avatar
  • 12.7k
5 votes

Trying Out DDD : How to enforce data integrity and immutability

First of all, thank you for the unit tests, it's very good to be able to refactor reliably. Let's start there. Testing code Test collection You can collect tests way shorter: ...
STerliakov's user avatar
  • 2,122
4 votes
Accepted

Instantiating View Models using a static factory

That class can still be abstracted public interface IAdSummaryMapper { List<IAdSummary> MapAdSummaries(List<AdResult> esResults); } The search ...
Nkosi's user avatar
  • 3,296
4 votes

Factory method to create two kinds of cars based on an enum

Do not name things with data type prefixes clsAutoFactory. No. Instead: AutoFactory Names should be what they are in the ...
radarbob's user avatar
  • 8,249
4 votes

PHP Factory Pattern : Should I use `create()` or `__invoke()`?

The only benefit of using __invoke() is that you can store the instance of the class in a variable as a callback and then use it without ...
Matías Pizarro's user avatar
4 votes

Electricity billing system for home in OOP

Attention to Detail What seem like trivial mistakes can be interpreted as a lack of attention to detail. So whilst your interface has a simple typo ...
forsvarir's user avatar
  • 11.8k
4 votes

C++ Template to implement the Factory Pattern

Seems pretty reasonable. I mean, I definitely wouldn't put this in production code because it relies on parsing a class name out of __PRETTY_FUNCTION__, and that's ...
Quuxplusone's user avatar
  • 19.7k
4 votes

Private VBA Class Initializer called from Factory

This is not so much an review but a variation of the OP's technique. As Mathieu Guindon mentioned in the comments the "logic needs to be encapsulated". My Caster class does just that. ...
TinMan's user avatar
  • 4,328
4 votes
Accepted

Creating multiple conditions in objects preventing nesting

You're probably overthinking this. Do you really need multiple classes? Unless those subscriptions have some distinct behaviour, there is no need for more than properties on just one class. Don't just ...
slepic's user avatar
  • 5,657
4 votes
Accepted

C++17 static inline member variables to populate factory at static initialization time

This is not safe It is very easy to make a copy&paste mistake which will cause problems later on. Consider: ...
G. Sliepen's user avatar
  • 69.3k
3 votes

Creating HTTP proxies for services

Too much indirection Your public methods, e.g. CreateMasterClient, already tell you which type the caller needs. When you design the shared private method the way ...
jpmc26's user avatar
  • 1,243
3 votes
Accepted

Model-to-Prefab mapper and factory

Your code mixes a factory and a builder by not using a consistent vocabulary which makes it confusing because one is not sure what it actually is. The main class is called ...
t3chb0t's user avatar
  • 44.7k
3 votes
Accepted

Choose betwen a few message values depending on 3 variables

This is how I would rewrite the code to be a lot more read- and maintainable. The most important point was the D.R.Y. (Don't Repeat Yourself) which was already pointed out by @mickmackusa When you ...
nick's user avatar
  • 46
3 votes

Instantiating a C++ class based on an enum value

I liked Deduplicator's however in the end it boils down to a one to one mapping of the enum and the tuple indizes which I think can be achieved fairly simply through std::tuple_element and a linear ...
Simon's user avatar
  • 31
3 votes

Chess move validator

I would like to add few more points to the previous answers with respect to design. If you are following the approach that chess board has the information about piece positioning, and piece is just ...
Ravi's user avatar
  • 171
3 votes

Creating a ModelBinder to Sanitize user input in HTML format

Developer gone 'rogue' I am not entirely happy with this code. The reason is, in the future, a developer may forget that HtmlSanitizerFactory should be used for instantiating HtmlSanitizer... so ...
dfhwze's user avatar
  • 14.2k
3 votes
Accepted

Java enum-based factory to calculate entry parameters

Your implementation is good, but I would suggest following points to improve: It is really hard to test/mock source2ToCheck.getSource(). ...
i.bondarenko's user avatar
3 votes
Accepted

Implement Factory pattern with multiple parameters and each parameters are interface

If I understand your intent and code properly then you rather have an abstract factory implementation than a factory method pattern implementation. The Abstract Factory pattern, a class delegates the ...
Peter Csala's user avatar
  • 10.8k
3 votes
Accepted

Factory Method Design Pattern Implementation as a Coffee maker program

Memory management and cleanup with inheritance Objects allocated with new must be cleaned up by calling delete. In this case the ...
user673679's user avatar
  • 12.2k
3 votes

Numerical integration in C++: Newton-Cotes formulas

Prefer Composition to Inheritance Currently, you have objects that store and hide the interval you’re integrating over, the tolerance, and the method of integration. Each class has a different ...
Davislor's user avatar
  • 9,115

Only top scored, non community-wiki answers of a minimum length are eligible