Hot answers tagged

25

private static final int RANDOM_NUMBER = 10; Everyone knows the true random number is 4. Less jokingly, what's the RANDOM_NUMBER for? Consider adding comments to explain the purpose of your variables... or better yet renaming them (to RANDOM_GRIDVALUE_MAX, perhaps). static Random rand = new Random(); public int numberColumns; public int numberRows; int [][] ...


24

Right, there's two parts to this review: Is what you are doing 'good'? Is there a better way? The answer to the second question is 'yes', but we'll have to explore your current code to understand why.... Current code, is it good? Your code, for the most part, is neat, well-named, and generally understandable. Great. But, there are some significant ...


22

related jsperf http://jsperf.com/12312412354 Well this is a really bad idea. Objects are always considered having different class if they don't have exactly the same set of properties in the same order. So a function that accepts these objects will in best case be polymorphic and in worst case megamorphic all the while you are thinking you are passing it ...


19

Consider the real world scenario a little bit. Does a car know what the speed limit is? Does it issue itself a fine if it's speeding? Or do the police do this? Assuming we're talking about the current state of things and not some near future world where cars do these things, police do. A more proper OOP approach would be to create a Policeman class with a ...


16

Try not to get in the habit of using using namespace std. Read this for more information. For clarity, have your #includes organized. Read this blog post or this answer for more information. Add a newline between each section of code. For instance, separate all user input and loops. For variables, it's best to initialize them late as late as possible in ...


15

I am a mathematician who hasn't written a real line of code in a decade, and not a computer scientist, so please take what I say with huge grains of salt! Just thought you might think my perspective is interesting. Why are you reducing so often? It seems computationally expensive. Of course you will want to reduce when the user asks e.g. what is my ...


13

I would do this using an interface and polymorphism to represent this: public interface ISession { void DoSomething(); // Add your properties / method definitions } public class ServerSession : ISession { private readonly string _serverName; public ServerSession(string serverName) { _serverName = serverName; } ...


13

Right, let me be clear, I mean this in the nicest of ways, but I don't like this code one bit. Let me start off a couple of simple things: Please, follow the coding standards as described by PHP-FIG I've noticed you're (ab)using the error supressing operator (eg @unserialize). Don't. Errors and Warnings are there to help you improve on your code. If there'...


13

Classes in Java should start with a capital letter, ex: PlayAgain rather than playAgain. If the class name is a verb like play again this means that this shouldn't be a class. Class names should usually be nouns like Person. If you feel that your class name should be a verb then it is a probably a function and not a class, ex: void playAgain(){ } Your ...


13

Field Visibility Restrict field visibility as much as possible. Use private final wherever you can (the compiler will let you know if you can't). Using this. in constructor In your constructor, you can use this. to assign your fields. This will allow you to use the same parameter names as field names. For example: public NumberGrid(int numberRows, int ...


12

Disclaimer I will mostly not critique what your class is doing, but mainly suggest how to make your interface as natural as possible (do as the ints do) by operator overloading best practices in terms of return type and const-correctness. I will mostly stick to C++98/03 and only sparingly suggest C++11 features. You could read the revamped GotW series for ...


12

Prefer nullptr to NULL if you're using C++11. <stdlib.h> and time.h are both C libraries. Use the respective C++ libraries <cstdlib> and <ctime>. The rest are okay. You don't need to include <vector> in main.cpp. Since you use a storage container for your deck, you can display it using iterators: for (auto iter = m_deck.begin(); ...


12

Student.cpp Initializer lists You should use initializer lists in place of these constructors. They offer some advantages, such as being able to initialize const members. Normal constructors do not allow that. The initializer list is in this form: Class::Class() : member(value) /* additional members separated by commas */ {} Here's what it'll look like ...


11

Enums Java enums have been around for a long time, so it's really a waste not to be using them at this point. However, I understand that this may simply be an exercise, to test and build your understanding of Java, one step at a time. If that is the case, (or maybe if the exercises are given in a specific order, and you haven't yet progressed to the ...


11

raw types Try to avoid raw types. private Node fieldname should be private Node<T> fieldname, same for function returns and method arguments. Bug If I do this: MyLinkedList<String> list = new MyLinkedList<>(); list.remove("foo"); I get a NullPointerException. This is because you first access data of the root, and then check what the ...


11

The overall object oriented design is not natural: A Person is a kind of Dealer? Normally it's the other way around: Person is the more general type, Dealer is a sub-type of Person. In real life, everybody is a Person but not everybody is a Dealer, right? A Person has a shufflePerson method... What does that even mean? If you put that in an English sentence,...


11

I am going to improve the following: allCars = [] x = 0 while True: allCars.append(Car()) print(allCars[x].plate + " was going " + str(allCars[x].speed) + "mph in a " + str(allCars[x].limit) + "mph zone") if allCars[x].speeding: print("speeding fine issued") print("\n") time.sleep(5) x += 1 Your x variable should be ...


11

Try to give your variables and types better names and maintain consistent naming. Better parse with regular expressions. fbValid is not good. It's better to prevent instances in invalid state to exist. When overriding operator, don't modify input values. Create a new instance (result of an operation) and return it. public class RationalNumber { ...


10

What happens if you change your database credentials? Right now your class is tied to a specific set of database credentials, and you'll need to change it if: you want to use a different engine, or a different host, or a different user, or a different password, or any combination of the above. This is why PDO requires you to pass the dsn and the ...


10

Looking at it from a general perspective... Put your comments that describe functions in javadoc-style outside your functions. Comment blocks before functions are a common format when it comes to describing a function. Because it's at the same indentation level as the function, it can't be mistaken as a comment about a specific section of your function. ...


10

For built-in types, there is no point in passing it as a const& - prefer a plain int (or a const int) instead. Other comments: Your add- functions are said to return int even though you don't have a return statement. I don't think that would or should even compile. Change them to void. Take the time to write out day, month and year. The extra seconds ...


10

Some general guidelines regarding #includes: only #include in the header when necessary (this reduces potentially unwanted dependencies) whenever possible, forward declare in the header instead of using #include whenever possible, #include in the implementation only (this does not affect dependency) never #include a source file (having to do so indicates a ...


10

Option 1 is surprising. I find it funny that you have return Enum.Parse(_propertyType, _parameter); in a parameterless Parse() method. Enum.Parse is just an example (int.Parse would be another); these methods are commonly used by C# programmers and your single-use parser seems to break POLS in my opinion. Option 2 is [more] consistent with the framework. I ...


10

A huge amount of that code is 'garbage', afraid to say. You already have all the basic pet data stored in the Pet class... so there is no need to duplicate it all in each pet type class (Dog/Cat, etc.). For example, your Cat class should remove all the duplication in then simply be: public class Cat : Pet { public Cat(string name, string breed, ...


10

Class Scoping So your comment afore your global declaration of e1 mentions how you wish for e1 to be accessible in all methods. If you want a variable to be accessible in all methods of a class, the conventional way is to assign it as a property of self: class Foo(): def __init__(self, bar): self.boz = bar * 3 def baz(self): # ...


10

Designing is art we learn from experience. Object oriented paradigm is "natural" way to explain things. SearchUser extending BaseUser is wrong naturally, isn't it ? (because SearchUser is not special type of user so can't be subclass) You can have has-a-relation ship there (i.e. SearchUser can have member which is object BaseUser ) BaseUser variables ...


10

Naming Class names in general should be singular - Group, Dot. If you have a constructor which sets coordinates x and y, call the parameters it gets x and y, not a and b... Also, I would refrain from single letter names, although, in this case, x and y seem ok, since they represent coordinates. Scoping You declared members, and implemented getters to ...


10

It's a fine starting point. Overall Design But if I was to classify it at the moment it really a property bag (a set of properties with no associated actions (thus not a class)). A class is a set of properties that have meaning together. The state of an object of that class is manipulated by actions that are associated with the class. Getter/Setters This ...


10

Class Program The removeSpaces() method is violating the single responsibility principle as it remove spaces from the given input checks for the word exit checks if the given expression contains a valid equotation So these 3 responsibilities should be extracted to 3 methods. Let us start with removing the spaces. We just use the Replace() ...


10

Not bulletproof Maximum amount of days in a month depends on both month and a year. It doesn't seem like the class cares about it. The rest of the answer might not be interesting to you. Operations on an object must be atomic What I mean by operation on object is getting the object from one valid state to another. Your default constructor violates this. ...


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