23

Edge cases are where such class hierarchies often fall short. An offhand short sword that adds to defense? A shield that does damage when you bash with it? A magic potion that not only makes you less hungry but also adds armor when you drink it? How will those fit into your current hierarchy? One way to avoid those problems is to use aggregation instead of ...


21

The problem, however, is that this means when creating my classes, I need to ferry around this TPos generic construct to places where it doesn't make sense to define it. I'm left creating new Player<Vector3> or Player<Int2> instances all over the place when really I just want to be creating a Player. Yes, specifying Player<Vector3> each ...


21

I am generally impressed with the consistency of the implementations, neatness, etc. I have a comment about the basic premise. The Shape should not be an abstract class, but an interface. It has no concrete implementations for any methods, and making it an abstract class makes it hard to inherit from other places too. Consider this instead: public ...


20

Firstly, I'm not sure you're actually implementing the requirement as it's written. The description says: An incoming call must be allocated to a fresher who is free. If a fresher can't handle the call, he or she must escalate to technical lead. This sounds like if there are no free freshers, the call shouldn't be handled at all (an exception thrown?), ...


15

Abstract cant be instantiated so having a public constructor for them is pretty much useless as they are being created through derived classes and it makes more sense to have a protected constructor instead of a public one public abstract class SpecialItem : Item { public virtual int GetSpecialProperty { get; } public override string ToString() { ...


14

I believe that in addition to the stated Java -> Scala OO question, that you may also be wondering about a good way to "organize" Models/DAOs/Controllers in Play 2. So I will incorporate that in my answer. I would have asked in a comment, but I don't seem to have the rep yet... A few notes before we get into an answer: fedesilvia is correct, that case ...


14

Your interfaces are inconvenient, and you are falling in to a trap of convenience that actually leads to inconvenience..... let me explain... C# makes it easy to create the getter/setter methods for interfaces/classes. There are three places I can see where your use (abuse) of this convenience leads to poor usability of your interfaces: public interface ...


14

This is well documented, well commented code. The docstring says: The one way in which slicing a WraparoundList differs from slicing an ordinary list is the case of using the list length as the upper limit. but this isn't quite the whole story — an ordinary list can also be sliced using a value greater than the list length, and in that case ...


13

Your example accesses the cat through a variable of type Cat and the dog through Dog. This would work, even if both classes did not implement a common interface. Inheritance is not involved. In order to really demonstrate the usefulness of polymorphism I suggest the following code: interface IAnimal { string Name { get; set; } string Cry(); } ...


13

Overall, code is easy to read. A few minor nit-picky items. Try not us use booleans, use enums instead. i.e. public enum EmployeeStatus { OnCall, Available } This allows you to add more status as requirements change: public enum EmployeeStatus { OnCall, Available, OutToLunch, OnVacation, OnBreak } I don't like that ...


13

Summary Use singular for class names. Use inheritance where appropriate. Follow code conventions. Be consistent about grammar. Write short methods. Use Singular for Class Names Use a name which represents a single object, not the whole group. I.e. change Dogs to Dog. An exception are utility classes, i.e. the utility class for arrays is called Arrays, ...


13

There's a lot of extending going on here. You should consider using interfaces, especially given the addition of default methods in Java 8. Also, some of your class names are strange: Sporting extends Dog and Working extends Dog would likely strike any onlookers as highly unusual. SportsDog and WorkDog would be better as class names but again, rather ...


13

Dynamic Memory Management Am I doing it right in terms of memory menagment? You should rather use smart pointers to store the Shape instances in the vector: std::vector<std::unique_ptr<Shape>> shapes; and std::unique_ptr<Shape> generateRandomShape() { int Shape; // Take care! Some compilers don't like it if variables ...


12

A nitpick on names: The dimensions of a rectangle should be width and height not width and length. The constructor of Triangle should take descriptive parameter names, such as side1Length. You have more excuse to use a and b for private fields (especially in this short, pure math code), but single-character names are generally frowned upon. Add javadoc ...


11

You might try this: myTechniques = [cls() for cls in Technique.__subclasses__()] Note that it does not return indirect subclasses, only immediate ones. You might also consider what happens if some subclass of Technique is itself abstract. Speaking of which, you should be using ABCMeta and abstractmethod.


11

A few points about your resolver. Since you're using an enum to specify the repository's Type and resolve the tools depending on that type, an EnumMap seems to be the simplest solution to resolving the specific tool implementation necessary. Also caching objects that have virtually no creation cost seems overkill to me. You can just eagerly initialize the ...


11

but have struggled with JavaScript Welcome to JavaScript, where all things look fine but are actually half broken. :D Finally I think I've found a way to 'create' classes that can inherit I think there's a saying that goes like "Composition over inheritance". That's because composition is more flexible and doesn't impose hard taxonomies of classes or ...


10

This answer will reflect the naming convention at my work. Naming Convention Prefixing I to all your interface seems superfluous, since we use a simple name like ContractService. The implementation class would have Impl suffixed to the name of the service like : ContractServiceImpl. Normally you will use DI to inject the implementation where you need it, ...


10

First, let us collect the code pieces which are repeated Fields private final TestHelpers helpers; private WebDriver driver; Constructor related helpers = new TestHelpers(); driver = helpers.getWebDriver(); Now, let's start refactoring by extending LoadableComponent<T> public abstract class ExtendedLoadableComponent<T> extends ...


10

200's answer is more or less on point and covers things from an idiomatic point of view, so I'd like to just address a bug in your code. It's actually a fairly major one: Your getXXX/setXXX methods are going to break your code because they reference globals. We'll use this one as an example: Animal.prototype.getOwner = function(){ return owner;} Animal....


10

Coal_ already made some good points. Let's just elaborate on the one about private. A class makes everything in it private by default that is why you don't need it if you order your interface this way. However it's recommended to order your interface from public to private so people who read it can see which methods you expose without having to read through ...


10

First, show your teacher the C++ Core Guidelines and various videos from conferences saying that you should not use naked new or delete anymore. std::vector<Train * > v = {&i, &r}; You are storing pointers to local (stack based) objects, which is correct in this example because the vector is in that same scope. But usually you will never do ...


10

int NextFlag(string category) I'd expect this to throw when it runs out of flags. I really really don't like that the first flag just happens to be 0: that depends on the order in which they are defined, and isn't written down anywhere. Option FromValue(int value, params Option[] options) I don't understand what this method is really meant to achieve... ...


9

Another thing to consider in your current design: ~Scene() { for (auto & f : v) delete f; } You are calling the destructor of the base here, not the derived classes' destructors. Remember to make a virtual destructor in your class to solve this problem.


9

As it is, I can see an oversight in the level_up function: imagine that you need 5xp to level up. You gain 50xp, you level up, and your xp is 0 again. You just wasted 45xp! Here is the corrected function: void level_up() { level++; exp -= req_EXP; // do not waste EXP req_EXP += level * 100; max_health += 50 * level; health = ...


9

General In many places, your formatting and alignment are inconsistent. You also have typos in your comments and even a couple of typos in the #endif portion of the header include guards. All of this needs to be cleaned up because it looks sloppy and makes the code harder to read. Adopt a consistent style—whatever it may be—and be disciplined about using ...


9

Once various responsibilites (functionality) is separated into appropriate classes, inheritance and using subclasses is not a problem. Rethink your core classes. Don't make client code do the work - that's a guarantee of duplication. A good class hides state and exposes functionality. Even basic functionality like overriding Equals pays off. "Do not make ...


9

Is this as extendable as I think it is? Does it work for multi-bit flags? For instance [Flags] enum Modifiers : uint { None = 0, Private = 1, Protected = 2, Public = 4, NonPublic = Private | Protected, // <- multi-bit All = ~None } Are there any APIs missing that I didn't think of or would be convinient? BitVector32 has ...


8

This abstraction is a leaky abstraction and should be avoided. As Mark Seeman puts it: Consider why you want to add IDisposable to your interface. It's probably because you have a particular implementation in mind. Hence, the implementation leaks into the abstraction. Now in this specific context he's talking about ISomeInterface : IDisposable, but the ...


8

Firstly, to address inheritance concerns, I use a t4 template to get around the single inheritance problem. ie. for every class where I need to introduce dispose support, I generate the an abstract base class that inherits from that class. Works a treat. Now, to your code. Here are the things that jump out at me: Constructors should be protected because ...


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