7

I'm having trouble extending an application. It is an attendance record system. Currently each employee records attendance by a card that has a QR code. Now they want to add fingerprint recognition, and there was no problem until they asked for two forms of identification must coexist in the system. Thus, the system must be able to sense QRs of the employee, but also his fingerprint.

I have the following classes:

The way to fix it in QrIdStrategy via .equalsTo(id) method is:

equalsTo(id){
  if (id == isKindOf (QrIdStrategy))
    if (this.getEmployeeId () == id.getEmployeeId ())
      return true;

  return false;
}

But I understand that asking about the class of an object is a bad practice and don't want to do that. How can I fix it?

I thought of the Visitor pattern, but I still have the same problem of comparing two classes that are not of the same type (because the system could scan any of the two types)

enter image description here

gal007
  • 6,911
  • 8
  • 47
  • 70
  • can every employee use both QR and fingerprint or are there some employees using only QR and others using only fingerprints? – Alexander May 03 '13 at 16:58
  • Every employee uses both – gal007 May 06 '13 at 14:12
  • 3
    I probably don't get what you mean, but why again would you have to test for the class of `this` if you're already in the `QrIdStrategy` class? – likeitlikeit May 06 '13 at 17:19
  • The code below *"The way to fix it in QrIdStrategy is:"* is of what method? And who might call it and to what end? – acdcjunior May 06 '13 at 23:18
  • as _ikeitlikeit_ said... why is there a need to find the class of `identificationStrategy`? is there anybody else who also uses the identification? like does a visitor need to identify? and is that what you want to see? is that what you want to find out? if yes then this is not the way you should do it... it looks like your problem is lack of understaning of OO. – max May 07 '13 at 12:59
  • Moki I really don't understans what are you asking me. Can you see the attached image with the UML diagram? – gal007 May 08 '13 at 00:05
  • @acdcjunior I edited what you said: "The way to fix it in QrIdStrategy via .equalsTo(id) method is:" – gal007 May 08 '13 at 00:09
  • @likeitlikeit Sorry, there was a mistake. It's this way: if (id == isKindOf (QrIdStrategy)) – gal007 May 08 '13 at 00:10
  • @gal007 ok since you said you don't understand me in the comments let me put it this way: you have an actor called employee who wants to identify herself using your system: 1. identification can be done using either of the methods but not both. 2. identification must be done using both of the methods so what is the way you want to implement your design? – max May 08 '13 at 09:28
  • @MoKi I really need to use both. I'm doing that in "bad way" now (asking object class), but I know there is an alternative! I'm thinking... – gal007 May 10 '13 at 00:38
  • Also on polymorphism, the Visitor Pattern is simply to address the lack of double (or multiple for that matter) dispatch in a language - a choice I think made for efficiency reasons (just like avoiding vtables if possible in C++). And while it might be considered bad practice to actively look for typeof in your code, some languages do exactly that under the hood (e.g. Scala Pattern matching on Case classes). – Ustaman Sangat May 10 '13 at 13:07
  • I may have misunderstood the problem when I wrote my answer. Each employee has both a QRCode and a Fingerprint. Does an employee need to identify using either *one*? Or do both need to be present at the "same time" constitute a valid, verified identity? – svidgen May 10 '13 at 18:44
  • @gal007 look at my previous comment and what svidgen said and specify. if both the methods are needed at the "same time" then the way you are trying to implement it is absolutely wrong and unnecessary. – max May 11 '13 at 09:25

3 Answers3

2

Maybe the first diagram already shows what I'm going to say, but this comes down to polymorphism.

Create an abstract class (e.g. IdentificationStrategy with some method like equlasTo()). Derive two classes: QRStrategy and FingerPrintStrategy from the IdentificationStrategy and implement equlasTo() method in each of them.

Now, there is a place in code where you know whether you are about to create an instance of QRStrategy or FingerPrintStrategy. Just assign an instance of that object to a variable of IdentificationStrategy type (e.g. let's name it 'foo' ). When you call the equlasTo() method on the 'foo', you don't care about object type any more.

user1764961
  • 673
  • 7
  • 21
  • But, before you call `foo.Equals(other)` you *must* ensure that `foo` and `other` aren't identical pieces of information. Or, you need something like `foo.IsNotEqualButIdentifiesTheSameThingAs(other)`. In other words, you can't be completely type-blind at auth-checking time. You need to ensure, for instance, that someone didn't just scan their badge twice. – svidgen May 10 '13 at 16:48
  • Yes, you can be totally type-blind, because having type information just for preventing scanning twice is very bad design. Class responsible for identifying users should do just that, identify them. On higher level you can store a flag saying if that particular user has been identified or not. And it doesn't matter how the user identified himself. – user1764961 May 10 '13 at 18:22
1

About your first soultion

The problem is that QrId and FingerprintId have really nothing in common. IdentificationStrategy shows that you want to compare them but you can't. The solution might be to add something like getUniversalID() : string to IdentificationStrategy and implement them in derived classes. Then you could have:

matchWithLocation(IdentificationStrategy identificationStrategy)
{
    return this.identificationStrategy.getUniversalId()
        == identificationStrategy.getUniversalID();
}

However, you might find implementation of getUniversalId (ensuring that IDs of different types will not collide) problematic.

About your second solution

A good adaptation of Visitor in this case would be adding methods getQr() : QrIdStrategy and getFingerprint() : FingerprintStrategy to Employee (since employee uses both methods of identification, as you mentioned), changing equalsTo(id: IdentificationStrategy) to check(employee: Employee). Then QrStrategy.check would be:

    return this.getEmployeeId() == employee.getQr().getEmployeeId()

and FingerprintStrategy.check would be:

    return this.getMatchingViaFramework() == employee.getFingerprint().getMatchingViaFramework()
lisp
  • 4,138
  • 2
  • 26
  • 43
  • I think to ask about the GlobalID is the same thing to ask the object class. :( – gal007 May 08 '13 at 00:12
  • not really. as you can see in the sample code of matchWithLocation, I don't mean the ID of the class, but universal EmployeeIDs that QrID and FingerprintID represent. – lisp May 08 '13 at 06:27
0

If I understand your qualm with the model as-is, it's functional, but it requires that you perform run-time type-checks on an instance of IdentificationStrategy -- and you'd like to avoid that. (Regardless of whether it's actually bad practice.) So, in light of the fact that every employee needs both, I'd suggest treating them as separate components of single Credentials object.

You can accomplish that in one of two ways.

The more generalized, perhaps more future-looking way, I think, is to assume that a user can have N means of identification, and that they need two of any to authorize. So, for each IdentificationStrategy object that comes in, you add it to a set of Credentials:

public class Credentials {

  public const int NumberOfDistinctIdsRequired = 2;
  private List<IdentificationStrategy> ids = new List<IdentificationStrategy>();

  public void AddIdentification(IdentificationStrategy id) {
    if (ids.Count < NumberOfDistinctIdsRequired) {

      // this .Contains() check should compare id's based on the raw, internal ID
      // or hash -- NOT by the underlying user/employee.
      if (!ids.Contains(id)) {
        ids.Add(id);
      }      
    }
  }

  public Employee AuthenticatedEmployee() {
    // if we have the required number of authentication pieces,
    // see whether they belong to the same employee
    if (ids.Count == NumberOfDistinctIdsRequired) {
      Employee rv;
      for (int i = 0; i < NumberOfDistinctIdsRequired; i++) {
        if (rv == null) {
          rv = ids[i].getEmployee();
        } else if (!rv.Equals(ids[i].getEmployee())) {
          return null;
        }
      }
      return rv;
    } else {
      return null;
    }
  }

}

Alternatively, treat one as an ID and another as a Password. I'd argue that, if the ID's are always presented in the same order, use the first-presented ID as the actual ID and the second-presented "ID" as a Password. In this model, your Fingerprint and QRCode are no longer both subclasses of IdentificationStrategy. And whichever is treated as the Password doesn't need to be able to return an Employee, but is rather fetched and verified against the Employee returned with the identifier. The identifier doesn't even need to be immediately present on the Employee (but probably is).

Let's assume the QRCode is the ID and the fingerprint "password." So ... do something like this:

public class QRCode : Identifier {
  public Employee GetEmployee() {
    // return the employee stored under the GUID, Int, String, or whatever
  }
}

public class Fingerprint : Password {
  public bool Equals(Fingerprint p) {
    // compare hashes or whatever.
  }  
}

public class Employee {
  public Identifier Id;
  private Password password;
  public Boolean IsAuthenticated = false;

  public Boolean Authenticate(Password p) {
    if (password.Equals(p)) {
      IsAuthenticated = true;
    }
    return IsAuthenticated;
  }
}

Or offload more of the work onto the Employee class. Or introduce a Credentials class that treats one as an ID and the other as a Password. Point is, they don't both need to be treated as forms of identification if both are always present. One can be used as an identifier and the other an authenticator.

svidgen
  • 13,744
  • 4
  • 33
  • 58