13

You said that you packed all the suggestions from the previous question, but there are still some pieces of advice that you did not integrate into your code. Here is how you can still improve your it: math.h legacy In your code, you are using the constant M_PI. While it will probably work on most of the compilers I know, this macro isn't defined anywhere ...


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(); } ...


11

You have to learn a lot about OOP... There is no polymorphism in your code, as this principle is designed to tackle exactly the kind of code you've written. The code that uses Asset should never branch its logic based on actual derived type (otherwise it would brake Open-Closed Principle), instead classes should use polymorphism to specify differences in ...


11

One edge case you're missing is how to handle null. isEmpty(null) will fail because Object.getOwnPropertyNames(null) will throw TypeError: can't convert null to object. Also be aware that isEmpty(NaN) will return false, which may or may not be what you want.


11

I see some things that may help you improve your code. Don't overcomplicate your code The only thing required of the Shape and Triangle classes is the calculation of area. For that reason, the code can be considerably simplified if the constructor calculates and stores the area. Use const where practical The Area() function probably shouldn't alter the ...


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

1) I am currently wondering if the following piece of Javascript code could be simplified? yes, something like this, would be more accurate and simpler to read var defaultValues = { "Array": [], "Boolean": true, "Object": {}, "String": "" } function getObjectProperty(object,field,type){ if (object.hasOwnProperty(field)) return object[field] ...


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 ...


8

A few random notes. I don't like the mix between "object oriented" and "procedural" here. If a Digit is supposed to "have a" position, then in no way should it be incrementing the y of that position during print (the digit doesn't move!). And, obviously, at print time, the Digit should not have to be reminded of its position each time (the position is a ...


8

Okay, after I worked my way through the original code, a few things have become clearer. Since I have never done programming with ncurses I was eager to try my hand at a better design. Here it comes. It's a sketch only in the sense that I didn't create separate translation units. That is basically a tedious exercise and left for the reader. However, it ...


8

For a junior hire, I may have accepted the answer, but I would have pointed out the short comings while discussing the answer, and see if the candidate could adjust the code accordingly. I agree with Edward's assessment that you should use std::domain_error for the exception, and that the Area method should be const. Missing Virtual Destructor As a matter ...


7

It stead of having a static method the basically does what polymorphism could do for you use polymorphism. public class Person { public string FirstName { get; private set; } public string LastName { get; private set; } public virtual void InitializeFromDictionary(Dictionary<string, string> row) { FirstName = row["FirstName"]; ...


7

After having called the method name("name") the type is lost so there is still an ordering to be respected: add((new Square)->name("s1")->side(1.)) will not compile. You could do that using a template as follows: template<typename TSelf> struct Figure { string _name; TSelf* name(const string & str) { _name=str; return (TSelf*)this; ...


7

I have a couple of issues with this: That is an ugly place to have a new operator, could it be avoided somehow? After having called the method name("name") the type is lost so there is still an ordering to respect: add((new Square)->name("s1")->side(1.)) will not compile. You should imagine to have many levels of inheritance and lots of ...


6

What you've written isn't really a "named parameter" API; it's more of a "getter/setter" API. Here's what it would look like in idiomatic C++11 — basically, changing all your new/delete heap traffic into plain old objects and using setName() for your mutator instead of just name(). #include <iostream> #include <cmath> #include <vector> #...


6

I disagree with the fundamental motivation here. Fluent interfaces are fine for builder objects, but constructing a half-baked class and initializing its fields with chained setters just feels wrong. The constructor should be enough to initialize the object. As for the pointer, consider using value semantics polymorphic objects through type erasure, as ...


6

The best solution is to make ComponentProperties know how to print itself. class ComponentProperties { friend std::ostream& operator<<(std::ostream& s, ComponentProperties const& d) { return d.print(s); } virtual std::ostream& print(std::ostream& s) const { return s << componentProperty_; ...


6

Listner needs a virtual destructor and the OnEvent() method should probably be pure virtual: class Listener { public: virtual void OnEvent(Event& event, Dispatcher& sender) {}; }; // Try class Listener { public: virtual ~Listener() {} virtual void OnEvent(Event& event, Dispatcher& sender) = 0; }; You need the ...


6

Problems with your interface design: The Employee interface you've designed is clearly lacking abstraction, since you have a dynamic_cast in the program to test if an employee is of a specific type. Employee should instead have a virtual method such as DoWork() which in turn will call Compiling() for Programmer, Selling for a SalesPerson and so forth: ...


6

I really don't like this enum and "type" member. public enum DataMessageType { Login, } public interface IDataMessage { ILogger Logger { get; } DataMessageType Type { get; } void WriteNetworkMessage(NetOutgoingMessage message); void ReadNetworkMessage(NetIncomingMessage message); } The idea behind polymorphism is that so long as the ...


6

Virtual destructor As a simple rule of thumb, if a base class contains any virtual functions, its destructor should also be virtual. The primary reason for it to declare a virtual function is so a derived class can override that virtual function. If you have public derivation, you nearly always want the destructor to be virtual--without that, destroying a ...


6

Your approach has at least two unfortunate aspects: Your approach to interface->object uses an extra heap-allocation. Adding a new "method" to int_triple_interface requires increasing the size of every "int triple" "object" in the program by sizeof(void(*)()). Nit: Your DESTRUCT(x) macro doesn't actually free(x). The first problem could be solved by ...


6

Downloader When I think of an ebook-downloader I see something like this: var downloader = new SyncfusionDownloader(); downloader.Download(ebookUrls, downloadDirectoryName); This means I'd like to create the downloader first and then tell it what to download and where to save it. I don't want to create a new downloader for each url or each download ...


6

For once it is nice to see code which is properly commented and for the most parts is easy to read and follow. Kudos to you for that. However, there are stuff which I'm wondering on why you've done as you've done, and with possible suggestions as to how you could tackle those. Starting from the top: Validate input, not necessarily all parameters – ...


6

[code] uint64_t It should be std::uint64_t, not uint64_t for key indexes (the latter is the C version). Or better: use std::size_t, which is the standard indexing type. [code] passing arguments At the moment, all the parameter pack arguments to the function calls are being passed by value. This should instead use perfect forwarding to prevent unnecessary ...


6

Why is LightSource inheriting from IMovable and IRotatable if not every LightSource is movable or rotatable? LightSource should only contain functions that relate to light sources, for instance intensity or color. The position and the direction are not properties of a light source here. Instead I'd make ParallelLightSource inherit from IRotatable and ...


5

As usual with your code, I can only come up with very minor nitpicks. With both push functions, there is a small chance of a memory leak. After the construction of a new Adapter<T> using: auto ptr = new Adapter<T>{ std::forward<Args>(args)... }; If entities.emplace_back(ptr); fails (say if relocation is required, the vector needs to ...


5

This seems like a good approach... Only got time for a quick comment but might come back later: protected void OnLoginMessageReceived(LoginMessageEventArgs e) { if (LoginMessageReceived != null) LoginMessageReceived(this, e); } The way you are raising the event isn't thread safe. This is the correct way: protected void OnLoginMessageReceived(...


5

I would use the adapter pattern (because I have suggested to use it), abstracting each needed class by an interface. In this way you add the ability to mock and test your code as much as you want. Also this is a lot more code (if you extract the classes to interfaces) but if your application is business critical, it should be testable on all possible ...


5

I want to keep my model without any depdency When one class uses another, that's dependency. OO programming is all about dependencies - we do not (should not) make single monolithic, do-all classes. However Decoupling direct dependencies is what all the 'oopla is about. System.Console.WriteLine("Name:" + child.Name + ", Age:" + child.Age + ", PocketMoney:...


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