40

For example, suppose I have a class, Member, which has a lastChangePasswordTime:

class Member{
  .
  .
  .
  constructor(){
    this.lastChangePasswordTime=null,
  }
}

whose lastChangePasswordTime can be meaningful absent, because some members may never change their passwords.

But according to If nulls are evil, what should be used when a value can be meaningfully absent? and https://softwareengineering.stackexchange.com/a/12836/248528, I shouldn't use null to represent a meaningfully absent value. So I try to add a Boolean flag:

class Member{
  .
  .
  .
  constructor(){
    this.isPasswordChanged=false,
    this.lastChangePasswordTime=null,
  }
}

But I think it is quite obsolete because:

  1. When isPasswordChanged is false, lastChangePasswordTime must be null, and checking lastChangePasswordTime==null is almost identical to checking isPasswordChanged is false, so I prefer check lastChangePasswordTime==null directly

  2. When changing the logic here, I may forget to update both fields.

Note: when a user changes passwords, I would record the time like this:

this.lastChangePasswordTime=Date.now();

Is the additional Boolean field better than a null reference here?

ocomfd
  • 5,652
  • 8
  • 29
  • 37
  • 51
    This question is quite language-specific, because what constitutes a good solution largely depends on what your programming language offers. For example, in C++17 or Scala, you'd use `std::optional` or `Option`. In other languages, you may have to built an appropriate mechanism yourself, or you may actually resort to `null` or something similar because it's more idiomatic. – Christian Hackl Feb 26 '19 at 08:53
  • 16
    Is there a reason you wouldn't want the lastChangePasswordTime set to password creation time (creation is a mod, after all)? – Kristian H Feb 26 '19 at 19:10
  • @ChristianHackl hmm, agree that there are different "perfect" solutions, but i don't see any (major) language though where using a separate boolean would be a better idea in general than doing null/nil checks. Not totally sure about C/C++ as I haven't been active there for quite a while, though. – Frank Hopkins Feb 26 '19 at 20:17
  • @FrankHopkins: An example would be languages where variables can be left uninitialised, e.g. C or C++. `lastChangePasswordTime` may be an uninitialised pointer there, and comparing it to anything would be undefined behaviour. Not a really compelling reason not to initialise the pointer to `NULL` / `nullptr` instead, *especially* not in modern C++ (where you wouldn't use a pointer at all), but who knows? Another example would be languages without pointers, or with bad support for pointers, maybe. (FORTRAN 77 comes to mind...) – Christian Hackl Feb 27 '19 at 09:30
  • I would use an enum with 3 cases for this :) – J. Doe Feb 27 '19 at 16:06
  • Are you familiar with the *null object pattern*? – Eric Lippert Feb 28 '19 at 08:30
  • Problems might occur when you need to distinguish between "the state of X is not known" and "X is known not to exist". But that seems not to apply to your use case – Hagen von Eitzen Feb 28 '19 at 12:57

8 Answers8

72

I don't see why, if you have a meaningfully absent value, null should not be used if you are deliberate and careful about it.

If your goal is to surround the nullable value to prevent accidentally referencing it, I would suggest creating the isPasswordChanged value as a function or property that returns the result of a null check, for example:

class Member {
    DateTime lastChangePasswordTime = null;
    bool isPasswordChanged() { return lastChangePasswordTime != null; }

}

In my opinion, doing it this way:

  • Gives better code readability than a null-check would, which might lose context.
  • Removes the need for you having to actually worry about maintaining the isPasswordChanged value that you mention.

The way that you persist the data (presumably in a database) would be responsible for ensuring that the nulls are preserved.

TZHX
  • 5,052
  • 2
  • 25
  • 32
  • 29
    +1 for the opening. Wanton usage of null is generally disapproved of only in cases where null is an unexpected outcome, i.e. where it wouldn't make sense (to a consumer). If null is meaningful, then it's not an unexpected outcome. – Flater Feb 26 '19 at 09:29
  • 28
    I would put it a little stronger as never have two variables that maintain the same state. It will fail. Hiding a null value, assuming the null value makes sense inside the instance, from the outside is a very good thing. In this case where you may later decide that 1970-01-01 denotes that the password has never been changed. Then the logic of the rest of the program does not have to care. – Bent Feb 26 '19 at 10:24
  • 3
    You could forget to call `isPasswordChanged` just like you can forget to check if it's null. I see nothing gained here. – Gherman Feb 26 '19 at 17:01
  • 9
    @Gherman If the consumer doesn't get the concept that null is meaningful or that he needs to check for the value being present, he is lost either way, but if the method is used, it is clear to everyone reading the code why there is a check and that there is a reasonable chance that the value is null/not present. Otherwise it is unclear if it was just a developer adding a null check just "because why not" or whether it's part of the concept. Sure, one can find out, but one way you have the information directly the other you need to look into the implementation. – Frank Hopkins Feb 26 '19 at 19:55
  • @Gherman IMO it provides a benefit in the *readability* of the code by providing more meaning/context — of course if you are writing new code against the class it doesn’t *force* you to use it. Giving the null-condition a meaningful name does more good than harm. – TZHX Feb 26 '19 at 20:04
  • @FrankHopkins he is not lost either way. Checking could be forced somehow. – Gherman Feb 26 '19 at 20:12
  • However, there is one potential drawback of a separate function (that applies to the original suggestion as well): concurrency. If you are running in a framework where concurrency is important and your value could change from having a value to having no value, you can easily run into a problem when first checking via the 'isPasswordChanged' method and, when it returns true, retrieving the object and without further check trying to access it. So you need to figure out first whether concurrency is an issue where this code is supposed to be used. – Frank Hopkins Feb 26 '19 at 20:12
  • 2
    @FrankHopkins: Good point, but if concurrency is used, even the inspection of a single variable may require locking. – Christian Hackl Feb 27 '19 at 05:30
  • @Flater I'd disagree there. If absence of value is a valid state Optional/Maybe is superior to just using `null` because it forces the caller to handle the no-value case properly. Returning `null` doesn't and someone can always forget to check for it and the bug might become apparent only after a long amount of time. Then: if you are writing in a language that does little to no compile-time checks than there isn't a big difference... but why use a statically checked language if you then decide to actively avoid the safety that the compiler could easily provide? – Giacomo Alzetta Feb 27 '19 at 08:14
  • @GiacomoAlzetta: The consumer will have to account for non-existence. Whether that's through a null check or an "exists" check is irrelevant - it requires an active effort and without it errors or unexpected behavior are always ready to occur. The issue with null is that it's being used excessively and often leads to a less meaningful null reference as opposed to a clear error (e.g. cannot find X), not that the usage of null should be avoided entirely. – Flater Feb 27 '19 at 09:21
  • @Flater: It's a matter of how easy the compiler makes it for you to do the wrong thing. Forgetting to check for `null` in Java is very easy, because the syntax of the programming language allows you to use the object anyway. Forgetting an `isPresent` check is not so easy, because you have to actively access the underlying object with `get`. IOW, `getPossiblyNullObject().doSomething()` is a potential runtime error, whereas `getOptionalObject().doSomething()` is a compilation error. You have to write `getOptionalObject().get().doSomething()`, and this piece of code already signals a problem. – Christian Hackl Feb 27 '19 at 09:53
  • 1
    @ChristianHackl yes, depends on what you want to do concretely, if you just want to make sure the consumer can properly operate on an object it retrieves, it would suffice if it retrieves it, does a null check and then operates on it; if it shouldn't operate on it when it has become null inside the component providing it, then it gets a lot more complicated. But I guess the details are out of scope for the question, yet, I'd find it relevant to point out that there are other problems waiting for you in a concurrent scenario, as it's often enough forgotten ;) – Frank Hopkins Feb 27 '19 at 10:32
64

nulls aren't evil. Using them without thinking is. This is a case where null is exactly the correct answer - there is no date.

Note that your solution creates more problems. What is the meaning of the date being set to something, but the isPasswordChanged is false? You've just created a case of conflicting information that you need to catch and treat specially, while a null value has a clearly defined, unambiguous meaning and cannot be in conflict to other information.

So no, your solution isn't better. Allowing for a null value is the right approach here. People who claim that null is always evil no matter the context don't understand why null exists.

Tom
  • 756
  • 4
  • 5
  • 18
    Historically, `null` exists because Tony Hoare made the billion-dollar mistake in 1965. People who claim that `null` is "evil" are over-simplifying the topic, of course, but it's a good thing that modern languages or programming styles are moving away from `null`, replacing it with dedicated option types. – Christian Hackl Feb 26 '19 at 12:24
  • 10
    @ChristianHackl: just out of historical interest - die Hoare ever say what the better practical alternative would have been (without switching to a completely new paradigm)? – AnoE Feb 26 '19 at 13:44
  • 6
    @AnoE: Interesting question. Don't know about what Hoare had to say on that, but null-free programming is often a reality these days in modern, well-designed programming languages. – Christian Hackl Feb 26 '19 at 13:55
  • 3
    This very much. I am currently rewriting a big chunk of one of our components to get rid of bad uses of null which cause the vast majority of our problems in production. If a method is to return a list, return an empty list instead of null, those kinds of changes. But there are plenty of places null is a perfectly acceptable valid value/state for a variable. – Hangman4358 Feb 26 '19 at 14:55
  • 2
    @ChristianHackl you are taking him out of context there. His "billion-dollar mistake" quote refers specifically to `null` **pointers**. On this I agree. Dereferencing a pointer and getting `null` is dangerous. But we aren't talking about a pointer here, we are talking about a variable value. – Tom Feb 26 '19 at 15:29
  • 10
    @Tom wat? In languages like C# and Java, the `DateTime` field is a pointer. The whole concept of `null` only makes sense for pointers! – Todd Sewell Feb 26 '19 at 16:09
  • 2
    @Tom What is the important difference between null pointers and null values? null values lead to the same problems of not checking for them – Gherman Feb 26 '19 at 17:18
  • 17
    @Tom: Null pointers are only dangerous on languages and implementations that allow them to be blindly indexed and dereferenced, with whatever hilarity may result. In a language which traps attempts to index or dereference a null pointer, it will be often less dangerous than a pointer to a dummy object. There are many situations where having a program abnormally terminate may be preferable to having it produce meaningless numbers, and on systems that trap them, null pointers are the right recipe for that. – supercat Feb 26 '19 at 17:19
  • 1
    @cupercat You convinced me that null pointers in C are indeed worse than null values in more controlled languages. But I know that not checking for null is one of the most common mistakes made by programmers. This is why I would want do something about them too. Programmers make the same misstake over and over again. Something should be done about that. – Gherman Feb 26 '19 at 19:17
  • @supercat It's still worse than just having an API that explicitly encodes the possibility of the absence of a value and that requires you to check for presence before attempting to use a value. Such an API can be provided in pretty much all modern languages, at no real additional cost over 'manual' null checks. – Cubic Feb 26 '19 at 21:01
  • 1
    @Tom: Languages like Java or C# go a long way to hide the truth from their users, but you are really dealing almost exclusively with pointers there. The OP still hasn't stated the programming language, but it's almost for certain that the `lastChangePasswordTime` variable is a pointer to a dynamically allocated object, i.e. that object itself is not included in the memory layout of the `Member` class. – Christian Hackl Feb 27 '19 at 05:34
  • @Gherman on a variable value, null is just another value. Sure you have to check it, the same way you need to check if something is zero before you divide by it. But if you forget to check, you'll just get either an implicit zero or a failed check. On a pointer, which is supposed to lead somewhere and thus can be dereferences, you get into memory access troubles if you don't check. – Tom Feb 27 '19 at 15:50
  • @ChristianHackl maybe that's my C background showing, but without contrary information I'd assume that `lastChangePasswordTime` is a timestamp, i.e. integer seconds since epoch. – Tom Feb 27 '19 at 15:52
  • 1
    @Tom: The code in the question sets `lastChangePasswordTime` directly to `null`, which wouldn't be valid in C, since `null` is not a valid keyword in C. In every language I know with a `null` keyword, `lastChangePasswordTime` would NOT be an integer, and would be a pointer. – Mooing Duck Feb 27 '19 at 18:38
  • 2
    @ToddSewell Note that in the case of C#, a `DateTime` is *not* a pointer; which is the reason for the existance of the nullable `DateTime?` type – gcali Feb 27 '19 at 18:56
  • @MooingDuck right, as I said, I was assuming from my C knowledge that other languages have timestamp / epoch times as well. – Tom Feb 27 '19 at 19:11
  • @ToddSewell DateTime is a value type in C#. EDIT: sorry, nevermind, someone already mentioned that. – Lakey Feb 27 '19 at 21:22
  • 1
    The problem with getting rid of nulls is that you are replacing code that crashed when it didn't check with code that does a likely wrong thing when it doesn't check. To me, a null should only be eliminated if you can produce a sensible result. – Loren Pechtel Feb 28 '19 at 04:52
  • @LorenPechtel: It's more like you are replacing code that crashed with code that doesn't even compile. – Christian Hackl Feb 28 '19 at 06:08
  • @Tom: Timestamps and epoch times aren't a feature of a language but of standard libraries or system calls, so of course other languages "have" timestamps and epoch times, too. It's just that in contrast to C, it's much more common to encapsulate those values for type safety, and in languages with poor or almost inexistent value-semantics support (Java is the traditional main offender here), that encapsulation in a class type automatically leads to allocation on the free store and thus a pointer. – Christian Hackl Feb 28 '19 at 06:42
  • @ChristianHackl yes, I understand that. What I don't understand is why people worry about type safety on an integer value, but that's a different discussion in a different place. – Tom Feb 28 '19 at 08:37
  • @Tom: Because it's not an integer but a date. That's the whole point of type safety. You would worry about type safety if someone tried to assign a `void*` to an `int`, would you? – Christian Hackl Feb 28 '19 at 09:45
  • @ChristianHackl yes, I get it. I get that some languages use a complicated DateTime object for simple things for no reason at all. And I said above that there's a difference between a pointer and a null value. But all this is going way out of bounds. – Tom Feb 28 '19 at 10:04
  • @gcali Thanks for the correction. – Todd Sewell Feb 28 '19 at 12:14
29

Depending on your programming language, there might be good alternatives, such as an optional data type. In C++(17), this would be std::optional. It can either be absent or have an arbitrary value of the underlying data type.

dasmy
  • 473
  • 4
  • 7
  • 8
    There's `Optional` in java too; the meaning of a null `Optional` being up to you... – Matthieu M. Feb 26 '19 at 11:22
  • 1
    Came here to connect with Optional as well. I'd encode this as a type in a language which had them. – Zipp Feb 26 '19 at 18:38
  • 1
    @MatthieuM. within languages that have null as a valid value globally, I find Optionals rather pointless and getting in your way more than helping, as a) the optional itself maybe null and b) even if something does not return an optional it could be null and just not be designed by the guy who likes Optionals. For those languages a mechanism like Nonnull annotation I find preferable as it doesn't try (and fail) to impose the reverse principle onto the language than it was designed with. – Frank Hopkins Feb 26 '19 at 20:07
  • 2
    @FrankHopkins It's a shame that nothing in Java prevents you from writing `Optional lastPasswordChangeTime = null;`, but I wouldn't give up just because of that. Instead, I would instill a very firmly held team rule, that no optional values are ever allowed to be assign `null`. Optional buys you too many nice features to give up on that easily. You can easily assign default values (`orElse` or with lazy evaluation: `orElseGet`), throw errors (`orElseThrow`), transform values in a null safe way (`map`, `flatmap`), etc. – Alexander Feb 26 '19 at 20:15
  • @Alexander yes it does have nice features. I've tried to use Optionals in existing code bases, but those features were absolutely rarely used, and "normal" checks with "isPresent" give nearly no benefit over == null checks; the main benefit of knowing what can and cannot be null is imho void because not supported this way by the language; afte all it was always more hassle than benefit and I'm now rather using annotation that goes the other way. For me this is now the only logical way given the language design. – Frank Hopkins Feb 26 '19 at 20:22
  • @Alexander but in my world teams are also not thaaat stable and there is quite some legacy code and external dependencies. If you have a private project with full control or a very fixed team you may gain more from it by using it via convention, but for me even that feels more like an attempt to turn the language into something it is not. – Frank Hopkins Feb 26 '19 at 20:23
  • 5
    @FrankHopkins Checking `isPresent` is almost always a code smell, in my experience, and a pretty reliable sign that the devs who are using these optionals are "missing the point". Indeed, it gives basically no benefit over `ref == null`, but it's also not something you should be using often. Even if the team is unstable, a static analysis tool can lay down the law, like a linter or [FindBugs](http://findbugs.sourceforge.net/bugDescriptions.html#NP_OPTIONAL_RETURN_NULL), which can catch most cases. – Alexander Feb 26 '19 at 20:29
  • @Alexander but you need to convert the code base first before any analysis tool can hard-alert, way too much effort for the benefit it gives. (Not to mention convince every new member it's a good idea). – Frank Hopkins Feb 26 '19 at 20:37
  • @FrankHopkins Yeah, although FindBugs has filtering (either in an XML config, or via Java annotations) that you can use to demarcate "legacy" code, for which you can ignore the warnings. That way, you can start incrementally improving the code base by writing "good" new code, checked by FindBugs, without failing builds because legacy code doesn't pass FindBugs. – Alexander Feb 26 '19 at 20:40
  • @FrankHopkins It's not pointless at all. Just treat the presence of any null anywhere as a bug. Any place where you explicitly assign null? Bug. Any place where you use an external API that doesn't conform to this without checking its result? Bug. – Cubic Feb 26 '19 at 21:03
  • 5
    @FrankHopkins: It may be that the Java community just needs a little paradigm shift, which may yet take many years. If you look at Scala, for example, it supports `null` because the language is 100% compatible to Java, but nobody uses it, and `Option[T]` is all over the place, with excellent functional-programming support like `map`, `flatMap`, pattern matching and so on, which goes far, *far* beyond `== null` checks. You are right that in theory, an option type sounds like little more than a glorified pointer, but reality proves that argument wrong. – Christian Hackl Feb 27 '19 at 05:28
  • @ChristianHackl I'd totally jump the Optional train if any of the next Java versions would shift the paradigm from everything is nullable to nothing is nullable and you need an optional where you want "null". I don't see this happening any time soon though. And with what we got, I find Optionals in reality get more in the way than help (in most real life Java projects). – Frank Hopkins Feb 27 '19 at 10:37
  • 1
    @FrankHopkins: You don't need the shift to "nothing is nullable", you need the shift to "nobody makes nulls because why should they?". Of course, this may never happen if the standard library itself doesn't lead by example. – Christian Hackl Feb 27 '19 at 10:47
  • 1
    @FrankHopkins C# 8 is moving that direction, so Java might get it within our lifetimes. – GalacticCowboy Feb 27 '19 at 20:30
11

Correctly using null

There are different ways of using null. The most common and semantically correct way is to use it when you may or may not have a single value. In this case a value either equals null or is something meaningful like a record from database or something.

In these situations you then mostly use it like this (in pseudo-code):

if (value is null) {
  doSomethingAboutIt();
  return;
}

doSomethingUseful(value);

Problem

And it has a very big problem. The problem is that by the time you invoke doSomethingUseful the value may not have been checked for null! If it wasn't then the program will likely crash. And the user may not even see any good error messages, being left with something like "horrible error: wanted value but got null!" (after update: although there may be even less informative error like Segmentation fault. Core dumped., or worse yet, no error and incorrect manipulation on null in some cases)

Forgetting to write checks for null and handling null situations is an extremely common bug. This is why Tony Hoare who invented null said at a software conference called QCon London in 2009 that he made the billion-dollar mistake in 1965: https://www.infoq.com/presentations/Null-References-The-Billion-Dollar-Mistake-Tony-Hoare

Avoiding the problem

Some technologies and languages make checking for null impossible to forget in different ways, reducing the amount of bugs.

For example Haskell has the Maybe monad instead of nulls. Suppose that DatabaseRecord is a user-defined type. In Haskell a value of type Maybe DatabaseRecord can be equal Just <somevalue> or it may be equal to Nothing. You can then use it in different ways but no matter how you use it you cannot apply some operation on Nothing without knowing it.

For instance this function called zeroAsDefault returns x for Just x and 0 for Nothing:

zeroAsDefault :: Maybe Int -> Int
zeroAsDefault mx = case mx of
    Nothing -> 0
    Just x -> x

Christian Hackl says C++17 and Scala have their own ways. So you may want to try to find out if your language has anything like that and use it.

Nulls are still in wide use

If you don't have anything better then using null is fine. Just keep watching out for it. Type declarations in functions will help you somewhat anyway.

Also that may sound not very progressive but you should check if your colleagues want to use null or something else. They may be conservative and may not want to use new data structures for some reasons. For instance supporting older versions of a language. Such things should be declared in project's coding standards and properly discussed with the team.

On your proposal

You suggest using a separate boolean field. But you have to check it anyway and still may forget to check it. So there is nothing won here. If you can even forget something else, like updating both values each time, then it's even worse. If the problem of forgetting to check for null is not solved then there is no point. Avoiding null is difficult and you should not do it in such a way that makes it worse.

How not to use null

Finally there are common ways to use null incorrectly. One such way is to use it in place of empty data structures such as arrays and strings. An empty array is a proper array like any other! It is almost always important and useful for data structures, that can fit multiple values, to be able to be empty, i.e. has 0 length.

From algebra standpoint an empty string for strings is much like 0 for numbers, i.e. identity:

a+0=a
concat(str, '')=str

Empty string enables strings in general to become a monoid: https://en.wikipedia.org/wiki/Monoid If you don't get it it's not that important for you.

Now let's see why it's important for programming with this example:

for (element in array) {
  doSomething(element);
}

If we pass an empty array in here the code will work fine. It will just do nothing. However if we pass a null here then we will likely get a crash with an error like "can't loop through null, sorry". We could wrap it in if but that's less clean and again, you might forget to check it

How to handle null

What doSomethingAboutIt() should be doing and especially whether it should throw an exception is another complicated issue. In short it depends on whether null was an acceptable input value for a given task and what is expected in response. Exceptions are for events that were not expected. I will not go further into that topic. This answer very is long already.

Gherman
  • 945
  • 1
  • 7
  • 13
  • 5
    Actually, `horrible error: wanted value but got null!` is much better than the more typical `Segmentation fault. Core dumped.`... – Toby Speight Feb 26 '19 at 17:57
  • 2
    @TobySpeight True, but I would prefer something that lays inside user's terms like `Error: the product you want to buy is out of stock`. – Gherman Feb 26 '19 at 18:10
  • Sure - I was just observing that it could be (and often is) *even worse*. I completely agree on what would be better! And your answer is pretty much what I would have said to this question: +1. – Toby Speight Feb 26 '19 at 20:35
  • 2
    @Gherman: Passing `null` where `null` is not allowed is a programming error, i.e. a bug in the code. A product being out of stock is part of the ordinary business logic and not a bug. You cannot, and should not try to, translate the detection of a bug to a normal message in the user interface. Crashing immediately and not executing more code is the preferred course of action, especially if it's an important application (e.g. one that performs real financial transactions). – Christian Hackl Feb 27 '19 at 09:41
  • @ChristianHackl I gave example code with handling `null` case. Handling null in such a case often is comprised of throwing a business logic-level exception. This is why I compared these errors. I am talking about avoiding programming errors, not just making errors better. – Gherman Feb 27 '19 at 10:13
  • @Gherman: I know that handling nulls and throwing exceptions if often confused with business logic. My point is that this is bad practice. – Christian Hackl Feb 27 '19 at 10:16
  • @ChristianHackl even without exceptions this is mostly some kind of an edge case requiring special handling. Quite often resulting in a message to user or to log. Often it's just a 404 http status. And value being equal `null` can be viewed as breaking the contract. It may be breaking of some restriction on some software component. – Gherman Feb 27 '19 at 10:39
  • @ChristianHackl I added a little bit on that topic in last section. I think it depends and I think my answer cannot fit required details. – Gherman Feb 27 '19 at 13:57
  • Good answer. I'm still not convinced by the "billion-dollar mistake", though. Every alternative to `null` I have seen brought the same problems than `null` or simply hid the logic away, with `null`s or equivalent in the background. – Eric Duminil Feb 27 '19 at 19:18
  • 1
    @EricDuminil We want to remove(or reduce) possibility of making a mistake, not remove `null` itself entirely, even from the background. – Gherman Feb 28 '19 at 12:55
4

In addition to all of the very good answers previously given, I'd add that every time you're tempted to let a field be null, think carefully if it might be a list type. A nullable type is equivalently a list of 0 or 1 elements, and often this can be generalized to a list of N elements. Specifically in this case, you might want to consider lastChangePasswordTime being a list of passwordChangeTimes.

Joel
  • 211
  • 1
  • 7
2

Ask yourself this: which behavior requires the field lastChangePasswordTime?

If you need that field for a method IsPasswordExpired() to determine if a Member should be prompted to change their password every so often, I would set the field to the time the Member was initially created. The IsPasswordExpired() implementation is the same for new and existing members.

class Member{
   private DateTime lastChangePasswordTime;

   public Member(DateTime lastChangePasswordTime) {
      // set value, maybe check for null
   }

   public bool IsPasswordExpired() {
      DateTime limit = DateTime.Now.AddMonths(-3);
      return lastChangePasswordTime < limit;
   }
}

If you have a separate requirement that newly created members have to update their password, I would add a separated boolean field called passwordShouldBeChanged and set it to true upon creation. I would then change the functionality of the IsPasswordExpired() method to include a check for that field (and rename the method to ShouldChangePassword).

class Member{
   private DateTime lastChangePasswordTime;
   private bool passwordShouldBeChanged;

   public Member(DateTime lastChangePasswordTime, bool passwordShouldBeChanged) {
      // set values, maybe check for nulls
   }

   public bool ShouldChangePassword() {
      return PasswordExpired(lastChangePasswordTime) || passwordShouldBeChanged;
   }

   private static bool PasswordExpired(DateTime lastChangePasswordTime) {
      DateTime limit = DateTime.Now.AddMonths(-3);
      return lastChangePasswordTime < limit;
   }
}

Make your intentions explicit in code.

Rik D
  • 3,806
  • 2
  • 15
  • 26
2

First, nulls being evil is dogma and as usual with dogma it works best as a guideline and not as pass/no pass test.

Secondly, you can redefine your situation in a way that it makes sense that the value can never be null. InititialPasswordChanged is a Boolean initially set to false, PasswordSetTime is the date and time when the current password was set.

Note that while this does come at a slight cost, you can now ALWAYS calculate how long it has been since a password was last set.

jmoreno
  • 10,640
  • 1
  • 31
  • 48
1

Both are 'safe/sane/correct' if the caller checks before use. The issue is what happens if the caller doesn't check. Which is better, some flavour of null error or using an invalid value?

There is no single correct answer. It depends on what you are worried about.

If crashes are really bad but the answer isn't critical or has an accepted default value, then perhaps using a boolean as a flag is better. If using the wrong answer is a worse issue than crashing, then using null is better.

For the majority of the 'typical' cases, fast failure and forcing the callers to check is the fastest way to sane code, and hence I think null should be the default choice. I wouldn't put too much faith in the "X is the root of all evil" evangelists though; they normally haven't anticipated all the use cases.

Mike Partridge
  • 6,587
  • 1
  • 25
  • 39
ANone
  • 311
  • 2
  • 5