6

If you let me to be frank, this is not a class but rather a ratatouille - that is a random collection of functions. I do understand your idea and a chain of thoughts and in a way you are trying to do the right thing - to achieve the main goal of every programmer - to reduce the amount of code written. And it's for the good you started with OOP. However, ...


5

A design pattern often used, is to have a non generic base type and a generic type derived from it or a type implementing a non generic as well as a generic interface. An example is public abstract class Comparer<T> : System.Collections.Generic.IComparer<T>, System.Collections.IComparer You can see the full implementation here public ...


4

Other developer-oriented issues such as variable namings, documenting, commenting, and such are not really a concern here since it's just for practicing algorithm and data structures, and have to follow the tutorial as much as possible. I feel this is a bad idea, this is as it promotes detrimental core habits. Myself and a friend used to play a purely ...


4

Or since the check functions build on previous functions should I structure it the following way This is not a case that makes you go for inheritance. You should go for inheritance to solve the problem of "repeating same properties in more than 1 class", for example, you have 2 classes Lion and Cheetah class Lion{ protected $speed; protected $age; ...


3

Use the force library Luke! Your code does use std::vector, but for reasons I don't understand, it also uses new to allocate a manually managed dynamic array as well. You don't seem to gain anything from this, so I'd advise using std::vector throughout. I'd also look at the standard library's algorithms. Some of them (e.g., std::remove_copy_if) could be ...


3

Coor has the comment coordinates ... yeah, that's exactly what the name should be then. But actually, Point seems easier and doesn't have to be abbreviated, or perhaps be more general and say Vector, or Vec2, that seems fairly common for games (despite it being an abbreviation). Not using the AWT class makes sense to me too. The hashCode method is okay, ...


3

I just have a couple of minor points to add to Olivier Jacot-Descombes' excellent answer. public override bool Faulty() { if (LogicalOperator == eLogicalOperator.Equal) return Value == Threshold; else if (LogicalOperator == eLogicalOperator.GreaterOrEqualThan) return Value >= Threshold; else if (...


3

Compiler error Unless you're using some experimental preview C# version, your code doesn't compile as this line: Value = singleValue ?? throw new ArgumentNullException(nameof(singleValue)); yields an error CS0019 Operator '??' cannot be applied to operands of type 'T' and '' You would need to add a constraint for that like: where T : class Nested ...


3

It's a bit counter intuitive you store Value as Object if the class IndexableOption<T> is generic. I understand you are trying to find a common type to store both T as T[]. However, if T : struct then what happens in case Value is T is a thing called boxing*. From reference source: Performance In relation to simple assignments, boxing and ...


3

Of #1 and #2, I prefer #2. But I actually dislike them both. For every customer, you are not only testing for the desired criteria, you are also testing the searchStringNotEmpty, locationsNotEmpty and selectedTagsNotEmpty flags. If you have 1,000,000 customers, that may be up to 3,000,000 tests of unchanging boolean variables. What you want is more like: ...


3

As far as which is easier to read, there isn't a clearly correct answer. The second is more shorter, but more cramped. I personally prefer code with a little more whitespace, such as the first example. Others prefer code that's more succinct. Which is more readable is probably a question to ask your coworkers. Marking variables as final will clue the reader ...


3

I can understand why no one has yet answered your question. The basic problem is that your User Class is not really an User Class. All it does is process user input, and that's not something an User Class should do. An User Class should deal with an user, nothing else. Things like: Changing user name and/or password. Verifying passwords. Keep information ...


2

Here are some suggestions. __DYNAMICS_H__ is a reserved identifier because it contains consecutive underscores. Use a non-reserved identifier like DYNAMICS_H_. Also, this name is too common and may introduce name clashes. One solution is to append a sequence of random characters. I see no reason to make Clonables a friend of Clonable. The latter doesn't ...


2

Your marker interface has no added value. In fact, it's an anti-pattern because you provide a generic type parameter on an interface ICommand, only to enforce it on another interface ICommandResult. public interface ICommand<TResultData> { } public interface ICommandHandler<TCommand, TResultData> where TCommand : ICommand<TResultData> ...


2

Docstrings You should include a docstring at the beginning of every method, class, and module you write. This will allow any documentation to identify what your code is supposed to do. Pulling from GeeksforGeeks, this is what a docstring should be: What should a docstring look like? The doc string line should begin with a capital letter and end with a ...


2

Overall your code is pretty clean. Abstract class There are a few different ways to do this. You can explicitly use the abc stuff, but the simpler thing to do is in your abstract methods (e.g. run), raise a NotImplementedError - at the Task level, not just KillTask. Then classes like KillTask that are effectively abstract children simply omit the ...


2

I read through your profile and saw you are a 17 year old developer hobbiest. I didn't know what OO was at that age. Well done. I made this review under the impression you were a somewhat experienced professional. AbstractSocketWrapper The constructor silently catches IOException. This leaves the socket in a corrupted state. Throw any exception up the ...


2

Docstrings You should have a docstring at the beginning of every module, class, and method you write. This will allow documentation to identify what your code is supposed to do. You're on the right track, having comments above the classes and methods. Now, just move those comments inside these classes and methods at the very beginning, inside triple quote ...


2

I don't quite know what lambda calculus is (I'm assuming it's a mathematical annotation for what we might call "purely functional programming"?), but I'll give this a quick shot. First, I'd love to have env populate itself if not provided. You really shouldn't have mutable default values for functions, though; the typical practice is to define: interp(self,...


2

In general, we try to keep functions small and nice, but in your case, using so many functions is just weird. For example, are you sure the uncrossedIntegersUpTo function is needed at all? How about determineIterationLimit? You can just handle them in the main function. Also, std::vector<bool> is not like the normal vector. It is not a container ...


2

Here're are my suggestions: Remove ConverterContract and ConverterActCompletion These two classes are unnecessary wrappers to hide the inner converter. You're saying that you're doing this to hide Fields and Dict so here's the next suggestion: Make Fields and Dict protected You don't want the outside world see them? Use proper access modifiers like ...


2

My observations... Leave generated classes alone You have modified Contracts by adding a custom constructor to it. You'll loose this changes when you regenerate the model. This class is partial not without a purpose. If you want to customize it then you should create another file in the same project and within the same namespace and define your own partial ...


1

Close on destruction You shouldn't need to explicitly close your file member on destruction. Since it's a safe variable and not an unsafe pointer, fstream's own destructor will be called and the file will be closed. Const members You should make path const, since it doesn't change over the course of the object's lifetime. File object scope inFile is not ...


1

Since writeTo does not modify v, it should take that parameter as a const reference (const std:::vector<T>&v). In LineEditor, the constructor doesn't initialize ary, so if you construct a LineEditor object and destroy it without calling removeLines (or if that function returns, possibly by an exception, before you allocate the memory) you'll ...


1

If all your configuration data are static, then why not using constants ? For instance : // A.h , without header guards for brevity class A { int x; int y; int z; public: A() : x(0), y(0), z(0) {} void load(); } // A.cpp const int A_config_x = 1; const int A_config_y = 2; const int A_config_z = 3; void A::load() { x = A_config_x; ...


1

Those are some very thorough class definitions, with many getters and setters. For context: I have some music experience (e.g. 22 years playing clarinet and saxophone, 18 years playing acoustic guitar) but wouldn't consider myself an expert in theory. I've gotten used to transposing from C to Eflat and Bflat in my head. I haven't really learned about coils ...


1

Single responsibility The account class is responsible for maintaining the account balance, generting account numbers and PIN codes. Instead of using primitive long and int types for account numbers and PIN codes, you should declare classes for both types and have those classes implement the validation. Logically, the Account class contains information ...


1

Interfaces, Contracts, APIs Your code do not follow the interface specified. Contracts/APIs may not be well-designed, and indeed usually are not; however they also usually are not modifiable. Therefore it is good practice put them in separate files such that you need not consult docs to see what you can change and what you cannot: public interface Accout {...


1

It looks very procedural at a glance, it might just not be useful to use custom objects here. In any case you don't need to explicitly inherit from object, class Process: is fine already. So, more maintainable, well, the headers look dubious, but obviously chosen from a real-life browser. The character set / encoding values would be the most concerning to ...


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