DZone
Thanks for visiting DZone today,
Edit Profile
  • Manage Email Subscriptions
  • How to Post to DZone
  • Article Submission Guidelines
Sign Out View Profile
  • Post an Article
  • Manage My Drafts
Over 2 million developers have joined DZone.
Log In / Join
Refcards Trend Reports Events Over 2 million developers have joined DZone. Join Today! Thanks for visiting DZone today,
Edit Profile Manage Email Subscriptions Moderation Admin Console How to Post to DZone Article Submission Guidelines
View Profile
Sign Out
Refcards
Trend Reports
Events
Zones
Culture and Methodologies Agile Career Development Methodologies Team Management
Data Engineering AI/ML Big Data Data Databases IoT
Software Design and Architecture Cloud Architecture Containers Integration Microservices Performance Security
Coding Frameworks Java JavaScript Languages Tools
Testing, Deployment, and Maintenance Deployment DevOps and CI/CD Maintenance Monitoring and Observability Testing, Tools, and Frameworks
Partner Zones AWS Cloud
by AWS Developer Relations
Culture and Methodologies
Agile Career Development Methodologies Team Management
Data Engineering
AI/ML Big Data Data Databases IoT
Software Design and Architecture
Cloud Architecture Containers Integration Microservices Performance Security
Coding
Frameworks Java JavaScript Languages Tools
Testing, Deployment, and Maintenance
Deployment DevOps and CI/CD Maintenance Monitoring and Observability Testing, Tools, and Frameworks
Partner Zones
AWS Cloud
by AWS Developer Relations
Securing Your Software Supply Chain with JFrog and Azure
Register Today

Trending

  • Integrating AWS With Salesforce Using Terraform
  • Revolutionizing Algorithmic Trading: The Power of Reinforcement Learning
  • Building a Flask Web Application With Docker: A Step-by-Step Guide
  • Scaling Site Reliability Engineering (SRE) Teams the Right Way

Trending

  • Integrating AWS With Salesforce Using Terraform
  • Revolutionizing Algorithmic Trading: The Power of Reinforcement Learning
  • Building a Flask Web Application With Docker: A Step-by-Step Guide
  • Scaling Site Reliability Engineering (SRE) Teams the Right Way

Code Review: Who Can Help Me

Oren Eini user avatar by
Oren Eini
·
Apr. 15, 11 · News
Like (0)
Save
Tweet
Share
2.66K Views

Join the DZone community and get the full member experience.

Join For Free

this time, taking on who can help me , i found a few really interesting things.

for one thing, we've got this little guy:

image

i was all ready to discover inewsrepository –<> newsrepository : repository<newsitem>, etc. what i found, instead was a real service:

image

to be frank, that's how it's supposed to be. there is a need to abstract something, and we write just enough code to make it work. i would argue with the implementation of this, however, because this particular approach to multi threading is wrong headed. we shouldn’t spin out a new thread pool work item just to execute the request when the fluenttwitter api already contains async version that can do quite well for us, but the overall concept is sound. and i was relieved not to find nested repositories in my first few steps there.

of course, i then discovered that this nice service has some friends from the left side of the blanket:

image

i think that i expressed my opinion about code like this:

image

a waste of keystrokes, and highly inefficient to boot. you don’t make queries by id in nhibernate, you use get or load instead.

overloading the infrastructure – after reviewing the code, i was surprised to see so much of it dedicated for caching:

image

what surprised me more is that in the entire application there were exactly two locations where a cache was used. in both cases, it led to the same service. implementing a much simpler solution in that service would have chopped quite a bit of code out of this project.

and then there was this:

image

luckily, this is dead code, but i was quite amused by this code, in a “shake your head in disbelief” fashion.

first, for a code that is obviously meant to be used in a multi threaded fashion, it is not thread safe. second, it is actually a memory leak waiting to happen, more than anything else. if you call that method, your items will never freed.

the next is a personal opinion, but i can’t help feeling that this is pretty heavy weight:

image

well, that is true whenever you are looking at an xsd, but in this case, we just need to expose two properties, and i think that it would have been perfectly fine to stick that into the <appsettings/> section. there are actually several places where similar approach has been tried, but i don’t see this of any value if you aren’t writing a library that might require special configuration. if you are writing an app, using the default modes is usually more than enough.

reading the controllers code wasn’t a real surprise. one thing that did bother me is the amount of mapping that is going on there, and how much of that i was simply unable to follow. for example:

image

which is then calling;

image

which then goes into additional custom implementations and convention based ones.

the reason that this is important is that this is a prime location for select n+1 issues, and indeed, we have several such occurrences of the problem just in this piece of code.

Published at DZone with permission of Oren Eini, DZone MVB. See the original article here.

Opinions expressed by DZone contributors are their own.

Trending

  • Integrating AWS With Salesforce Using Terraform
  • Revolutionizing Algorithmic Trading: The Power of Reinforcement Learning
  • Building a Flask Web Application With Docker: A Step-by-Step Guide
  • Scaling Site Reliability Engineering (SRE) Teams the Right Way

Comments

Partner Resources

X

ABOUT US

  • About DZone
  • Send feedback
  • Careers
  • Sitemap

ADVERTISE

  • Advertise with DZone

CONTRIBUTE ON DZONE

  • Article Submission Guidelines
  • Become a Contributor
  • Visit the Writers' Zone

LEGAL

  • Terms of Service
  • Privacy Policy

CONTACT US

  • 600 Park Offices Drive
  • Suite 300
  • Durham, NC 27709
  • support@dzone.com

Let's be friends: