0

Is this a code smell?

I have a repository which connects to a SQL server. This repository uses Dapper to map the results to an object.

I have a method in this repository that checks a result, then passes a bool back on whether the result passed a test.

Since I don't want to use the results of the SQL query outside of the method, I want to create a private model inside the repository file to act as a middle-man, something the query can map to temporarily, whilst the rest of the method runs whatever else it needs to.

I don't want to make a public method as it won't be used outside of this repository, I'm also actively trying to reduce the amount of files this project has.

Is it bad to use a private class inside a repository file? Does it need to be a public class in a model folder?

2

No, it's not bad to have private classes in repositories.

I guess the confusion would be around why you need a private class. Rather than another option.

  1. Use an existing public class.

    Your Repository is a conceptual storage for public classes, If I need to ask a question about the state of that data, can I not use the public class it maps to?

  2. Don't map to a class at all

    If the question is about the stored data as a whole. eg "are there more than 100 customers stored?" Then can I not map the answer of my query to a simple value type, or do the entire calculation in SQL and return a boolean?

    Or, If the question is complex and needs to be done in code can I not use a DataSet rather than mapping to an object?

  3. Why private rather than internal?

    Rather than a private class, it might be better to use an internal class. This allows you to put the class in its own file. I know you don't like files, but its the standard way and the next program to look at the code will thank you for it.

I should add a clarification. Its normally a good thing to map complex data into classes and encapsulate the functions as methods.

What you should try to avoid is making data classes for the results of random queries. Because these have no conceptual object they align with they can multiply out of control as you add and tweak various ad-hoc queries.

If you do need to have such a class then keeping it private or internal is a good thing

  • 1. It's returning a result from multiple joins, column alias etc. 2. That's interesting, I've never used DataSet before, maybe this is the time to start. I'll look into it, thanks! – Jessica Ward Dec 10 '18 at 10:09
  • 3. Regarding your last point, that's why I'm asking this question. I know files are important, it's not that I don't like using 1 file for 1 class, it's more that I'm trying to remove some unnecessary classes altogether. I don't want to throw 2 classes into one file, so I'll look at an alternative, like the DataSet example you provided. – Jessica Ward Dec 10 '18 at 10:11
  • If you have a simple function and just need a bit of code rather than doing it all in sql, then a dataset might be the way to go. you dont need a mapped class just to see the results of a query – Ewan Dec 10 '18 at 10:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.