last day (15 days later) » 

20:22
5
A: Employee Attendance structure design

Morten BorkI would change my parameters to simply require "login" and "logout" timestamps. Then calculate the difference between those two DateTimes. I would move all validation into DI behaviour, so it's easy to test independently, and change, based on PO or customer feedback. That should clean up the mess...

but because there are two input options: interval(login/logout) or hours. When only hours are provided, timestamps are not provided so they are null. is it better to have separate constructors for creating attendance with hours and timestamps? Because attendance will always have date (for which day it represents) and either will have hours field as not null or intervals field as not null.
It is likely better to create a different implementation of IAggregateRoot that accepts only hours, and one that accepts only to/from. because the logic and handling of the data and thus the responsibility of the class are likely to be different. Like uncle bob says, a class should only have 1 reason to change. (You could create a factory that determines which parameter is used and provides the correct implementation of IAggregateRoot
Should these two separate classes be mapped to one database table or should they be contained in separate tables as well?
Depends on your architecture, specifically your application domain: Lets say that the reason you can receive hours as a parameter, is because there is a specific hardware that only tracks hours spent on something, and this hardware is used to provide this parameter, then it would make sense to store the data that is specific for that hardware in it's own table. And, lets propose a punch clock tracks logins and logouts, for duration calculation, then store those timestamps in a different table would make sense. However, if the domain, is ONLY to aggregate over hours, then a single
table is fine, and you can simply store (to-from).hours, together with "hours" domain model as one DB model The trick is to understand how and why something will be used, so you can model your code to appear as close to reality as possible, and by "reality" I mean, as close to how the code will accomplish something for the real world, not by actual reality.
But if you have to store Datetime from, DateTime to, as null, when only hours is provided, consider your model carefully, aren't you trying to solve two responsibilities at the same time, if you can disregard half the properties in your class? If they are optional, doesn't that mean that you are technically doing two things in one class? That is usually not desirable
As a general rule of thumb, you want one reason to change your code. If the hardware of your hours device changes, should I need to review and retest the code of the punchclock? Why? That would be annoying. Segregate responsibility as a rule, unless you see the actual usage, "hide" the differences from both the system and the user. In my own personal opinion, I would happily segregate too many responsibilities, rather than too few. Fixing a high coupling low cohesion code base is always worse than the opposing scenario.
Last comment, make sure your clean code rules are also maintained, if you segregate code a lot, but don't use proper clean code techniques, you can end up making your code more confusing. The number 1 metric of good code, is: "Can other people easily understand my code". If the answer is yes, then this code is good, even with the WORST performance since 1945. Because, someone can understand what the code is meant to do, and help you fix your under performing code. If they cannot understand your code, they can't help you.
C# interfaces cannot declare constructors. Thus create a different implementation of IAggregateRoot that accepts only hours is impossible. Besides, this interface declares only properties. We know this because Attendance : IAggregateRoot class has no methods.
Creating a different interface - IShiftValidator - and implementing it in a separate class - together and separately breaks the coherence of the AttendanceFromTo ( or Attendance ) class. Validating Attendance constructor inputs in a different class? No. Just no. Given that, a separate interface definition , even if implemented in `Attendance, is pointless.
Referring to times/dates is usually stated as "earlier than" or "later than", rather than "less than" or "greater than". Yeah, I know. It's an exception, "not for customer consumption" kinda thing. Just sayin'
20:22
@radarbob I dont think you understand the qoute you wrote: an implementation of an inteface, is a class, that meets the requirements of an interface. Thus the implementation can have a constructor....
@radarbob So you only need to do a validation that a shift is valid on the attendence class? You have no idea what you are talking about.... And it isn't, you can unit test shift attendance and DI it in anywhere you want, and you do not have to retest any logic in Attendance, if the validation requirements change. Also it does not break the coherence of the class, you have actually follow SOLID, you have made sure you implementation has 1 reason to change. You write bad, unreadable code my friend. Sorry.
@radarbob constructor name didn't get updated that was all. I do not wish to rewrite his entire example, so I will only show what I mean with examples where relevant.
We do not know what IAggregateRoot looks like but I assume some or all are reflected in Attendance constructor parameters. In this solution I don't know how to reconcile the interface and 2 different constructor signatures without explicitly defined constructor parameters. .... The OP's suggestion of constructor overloads seems a better alternative.
@radarbob You create a factory, that determines if you have hours or to/from, depending on which it is, it returns an implementation of IAggregateRoot ?
no idea what you are talking about => No where did I say or suggest that constructor parameter validation is not needed. Separating it from the class it belongs to is de-facto bad design.
@radarbob ShiftValidation, does that have anything to do with Attendence? Are those responsibilities in high cohesion with each other? NO. Do you know how that is immediately evident? Because it is simple to abstract the responsibilities out from the class, and DI them in. You can infact do so, with any DateTime to/from, that do not belong to attendance, and get valid results out from the method. Thus, say you want to validate from/to values other places in the code, which is frankly, likely, you would then to also implement a shift validation there. Why? That is duplicate code.
@radarbob it is infact POOR design, to create high coupling between responsibilities that are not related, requiring you to test or change more classes from a single responsibility change. If you follow your idea, you would have to change shift validation, through all classes that has such an implementation, and you would have to retest all classes with such an implementation as they are all highly coupled. if you DI in the shift validation, not only does it become clear what Attendance is responsible for, it also is clear what it isn't responsible for.
Also you aren't hiding it, you are clearly dependency injecting the method in, so you are clearly telling a review, I also validate the shift in the attendance class. This is clean code and SOLID. And you only test one class to validate that a change to shift validation is valid.

last day (15 days later) »