• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
  • Liutauras Vilda
  • Campbell Ritchie
  • Tim Cooke
  • Bear Bibeault
  • Devaka Cooray
  • Jeanne Boyarsky
  • Knute Snortum
  • Junilu Lacar
Saloon Keepers:
  • Tim Moores
  • Ganesh Patekar
  • Stephan van Hulst
  • Pete Letkeman
  • Carey Brown
  • Tim Holloway
  • Ron McLeod
  • Vijitha Kumara

OOP, looking for advice/critique  RSS feed

Ranch Hand
Posts: 31
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I'm not sure if this is allowed, since I do not technically have an error popping up. Yesterday I posted a "thats wrong OOP" and got some constructive advice.
From it I understood that my approach to it was wrong and after a bit of thinking and learning today I came back hoping to receive advice again.

Hope I'm not breaking any rules or something. I'm just honestly enthusiastic since I think that something clicked in my brain and I have a better understanding now.

Line of thought this time:

Company has offices, departments and of course employees that work on projects that are assigned to teams in some departments. (all that UML to one sentence)

Any critique/advice is welcome and appreciated.

Thanks in advance!
Saloon Keeper
Posts: 9248
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
  • Using concrete types instead of interfaces, e.g. ArrayList instead of Collection.
  • Methods don't show parameters.
  • Company.getDepartments() returns String.
  • No way to add departments or offices to a company.
  • Department has a getDepartment() method?
  • Team has a getTeam() method?
  • Employee has a getEmployee() method?
  • Inconsistent naming: changeManager() but setTeamLeader().
  • The properties of Position have nothing to do with a position at a company.
  • Is a building number always an int?
  • Times should be represented with LocalTime, not int.
  • Many private properties that are unrelated to public API.
  • Many properties expressing relationships that are not represented with connections in the diagram.

  • I suggest you design your public members well before you add private ones. Private members are usually just implementation details, and not appropriate for an UML diagram expressing a first design.

    Another improvement I suggest you make: for many-to-one relationships with "entities" (Employee) rather than "values" (Address), use a Map rather than a Set or List. An Address can be uniquely identified using its properties, so you can add it to a Set. An Employee can not, unless you add an id field. Map entities by their IDs.
    Don't get me started about those stupid light bulbs.
    • Post Reply Bookmark Topic Watch Topic
    • New Topic
    Boost this thread!