The first version of pseudo code I consider a code smell because of (I think) the Command Query Separation principle and/or the S in S.O.L.I.D. What I like to see is in the 2nd version.Should I keep dinging these in code reviews or let it go because everybody seems to do it?#1st:versionClass ServerCollection {   Server|null: getServerWithIP(IP)}#2nd:versionClass ServerCollection {  boolean: hasServerWithIP(IP)  Server: getServerWithIP(IP)}
Is defining only one method against S in S.O.L.I.D
code quality;patterns and practices;solid
The first version is identical to how indexed or keyed collections with lookup are often designed: trying to retrieve a non-existing value will return some value denoting the absence of a value, e.g. null or nil. For a generic container that's undesirable because it means you cannot distinguish between a value that is not there and a value that is null. However, for your case, that won't be a problem, since null is not a possible value, you only have Server objects.Another possibility is throwing an exception if the value isn't found. But, looking up something in a dictionary and not finding it is not an exceptional situation, it is quite normal.In the .NET world, there exists the Try pattern: bool TryGetServerWithIP(IP ip, out Server server) will return a boolean indicating whether not the the server was found, and the actual value will be available in an argument passed by reference:Server myServer;if TryGetServerWithIP(new IP(192, 168, 0, 1), myServer) {  // you can use `myServer` here}But really, the best way IMO, is to return an Option type (also known as Maybe):Option<Server> GetServerWithIP(IP ip)And in addition, it is also often nice to have this:Server GetServerWithIPOrElse(IP ip, Server default)If you are not familiar with Option or Maybe types, this is basically isomorphic to a container that is either empty or has exactly one element. It is pretty nice for such an Option type to implement the standard container API (e.g. IEnumerable in .NET or Iterable in Java). This allows you to, for example, use the language builtin foreach loop to process the value only if it exists. Also, Maybe can be a monad, which is again pretty nice just for the monadic properties but even more so for languages which have builtin support for monads, such as C#.