9
Graph seems quite reasonable:
But your distance implementation is not an implementation of Dijkstra.
Dijkstra finds only the cheapest path between two nodes (technically).
But I suppose you can use the technique to find the distance from a node to all others nodes. Dijkstra works by finding the cheapest distance to a node then expanding from there (it never ...
8
using namespace std
It is bad practice to have this in your program. Sometimes it is OK if you are writing a small program for something like a programming challenge. Otherwise, I highly discourage the use of this in your programs. The std namespace is HUGE and writing this statement means that now you have no idea what is a part of the standard library and ...
6
When you're comparing against 8 different patterns, and then simply removing - for 7 of those validations, why not just remove - initially, and then validate against the only remaining pattern.
Another thing to note is, in the patterns, at the end; you have a character set: [\d|\X]. This actually will match one of:
a digit
literal |
literal X characters (...
6
Overall Observations
The program would be simpler if the following functions were members of the Person Class:
void AddPerson(Person& objPerson)
void DoYouPayAnnualIncomeTax(Person& objPerson)
void DoYouPayStudentLoan(Person& objPerson)
void CalcBandIncomeTaxPayer(Person& objPerson)
void DisplayResults(Person& objPerson)
In that case ...
5
Wrap your class in a namespace
Just like the STL has its containers in the std:: namespace, it's a good practice to wrap your containers in your personal namespace too. That way you group related classes and functions together.
Node<T> Node
template <typename T>
class DoublyLinkedList
template <typename T>
struct Node {
T ...
5
Personally, I would suggest that you should have two classes, not just one and some static functions.
Consider the extensibility of this for the future.
If you have class person and another class taxregime then you have the makings of useful program. As someone else has said, all the constants that you use should be configurable. By having the taxregime ...
5
Others have mentioned the scattering of magic numbers through the code, and suggested that you give names to them. I'd consider going further than that, and make the tax rules as much as possible be data-driven. In the Real World, governments tweak the thresholds pretty frequently, and you wouldn't want to have to re-compile versions of your program with ...
5
hjpotter92 starts with the right idea:
Unify and simplify.
You just should go much further.
Also, don't give up if the last digit is X, that's a perfectly fine digit for your code.
You aren't (or shouldn't) be interested in all the dashes, so ignore them.
Could you even know that you have all the used patterns?
Take a closer look at wikipedia on ISBN10, and ...
4
This is quite a decent implementation of a C++ container! Some minor improvements are possible though:
Prefer using default member initialization
Especially when you have multiple constructors, default member initialization saves typing and reduces the chance of accidentily not initializing a variable. So:
class Queue {
public:
Queue() = default;
...
4
Avoid convenience aliases unless it really improves the code
You created the type alias enable_, but it is only used twice. Furthermore, if you wouldn't have seen the alias, and first see enable_<trigger>, then you would first have to find the definition of enable_ to know what this expression does, whereas if you wrote std::enable_if<trigger, void&...
3
The InsertAt() and RemoveAt() operations are quite expensive
when implemented to accept a position. These operations are
usually implemented using iterators, which are wrappers around
the internal node structure.
Making the stream output operator part of the class is rather inflexible. You should define a standalone non-friend function to help you ...
2
Instead of DisplayResults, opt to overload the << operator for your User class
std::ostream& operator<<(std::ostream& os, User& const u) {
os << "***" << '\n';
os << "Weekly wage: " << u.WeeklyWage() << '\n';
os << "Yearly wage: " << u.YearlyWage() <...
2
To address the locking overhead in the single-threaded case, my first advice is (as with all performance questions) to measure before optimising! It's likely that the locking overhead is tiny compared to the queue's memory management.
If you really find you need to streamline the locking, then you'll probably want to have two versions of the engine. The ...
Only top voted, non community-wiki answers of a minimum length are eligible
