c# - object == object instead of object.id == object.id potential problems -

i have inherited sloppy project , tasked explaining why bad. i've noticed on code have done comparisons this

(iqueryable).firstordefault(x => x.facility == facility && x.carrier == shipcode.carrier 

in example above x.facility database , facility shipment.facility mapped complex object in nhibernate.

i have expected see .firstordefault(x => x.facility.id == facility.id

at first thought comparing whole object might cause issues if facility record changed in db facility stored in shipment different.

after thinking more realized shipment.facility populated id should match if facility record changed.

it still feels wrong me see being buggy , hard track down? there somthing wrong comparing entire object vs id.

expanding previous comments answer:

i think good practice when orm allows it. i'm not experienced nhibernate, rest of answer i'll assume in fact implement equality in sensible way (such comparing primary keys). should ensure case, otherwise code not bad potentially buggy.

as analogy, forget sql moment, imagine dealing poco not part of orm. want choose between following:

approach 1: iequatable

public class facility : iequatable<facility> {     public int id {get; private set;}     //the rest of properties      public bool equals(facility other)     {         return other.id == id;     } } 

(you'd want override object.equals, i'll exclude brevity)

approach 2: iequalitycomparer

public class facility {     public int id {get; private set;}     //the rest of properties }  public class facilityidsmatchequalitycomparer : iequalitycomparer<facility> {     public bool equals(facility x, facility y)     {         return x.id == y.id;     } } 

(gethashcode excluded brevity).

which of 2 approaches better? well, i'd it's approach 1 clear margin. adheres 2 important principles approach 2 doesn't:

• don't repeat yourself. in second approach, code anywhere trying compare facilities have use facilityidsmatchequalitycomparer. fact, particular equality comparison logic needs used, sprayed on solution, repeated every time want compare them.

• single responsibility principle. not every class doing comparison need repeat code, code taking on responsibility doesn't belong class. should facility express implementation of equality, not every class wants use it's done comparing ids. (yes realise make it, say, facilityequalitycomparer calling classes remain agnostic how equality comparison done, purpose of analogy code in op, exact comparison logic hard coded every comparison)

so, bringing actual question, analogy closely mirrors situation have here. it's not quite simple facility implementing iequatable, principle same: orm taking own responsibility how equality checking implemented, rather pushing responsibility out code using it.

this means if i'm writing code, outside of data access layer, can "i want check if these 2 objects equal, i'll write object1 == object2", rather "i want check if these 2 objects equal, , these 2 objects entities, because of way implement our persistence means when check equality converted sql query, need write check if writing sql, means need compare primary keys, either checking attributes or maybe through knowledge of conventions in data access layer, know means comparing id properties. i'll write object1.id == object2.id".

orms aren't perfect, , can't abstract away underlying sql database, when can, should!