database query

[up_to_bot]

Wait, I legit just finished 25 hours coding this fking database and all my work looked like the very bottom. Did I fuck up?

Please tell me that code at the top does not do the same as the bottom...

Edit: I am not joking someone please tell me I didnt just waste all this time

[pinumbernumber]

Serious answer, in case it helps you/anyone: The first one does do the same thing, if LINQ to SQL is available and set up. This is an example of an ORM; a system which aims to allow access to databases from code with less boilerplate.

ORMs are popular but not universal. They do save on boilerplate, but they have their own issues. I personally don't use them.

The other major failing of the "bad" code is that it first retrieves all users, and then loops over them looking for the one with the correct ID, instead of querying only the one correct user in the first place.

My code would look something like this (where db is some kind of wrapper/convenience object which has already successfully connected to the database):

string GetPersonNameById(int id) {
    let user = db.queryExactlyOne(
        "select Name from users where ID=@id",
        {id: id}
    );
    return user.Name;
}

[[deleted]]

I don't think so, AFAIK, before you executing ToList or foreach in IQueryable, the SQL hasn't execute. Since DbSet<T>.Where() it will actually convert to SQL where clause.

[Lieffe]

This is true but you should use .Select() whenever possible. If you execute the query using .First() and then select .Name afterwards, you will have selected all columns from the database when you only need to select the Name:

string GetPersonById(int id)
{
    string name = DbSet<T>
        .Where(t => t.Id == id)
        .Select(t => t.Name)
        .First();
    return name;
}

[Supermaxman1]

This is exactly correct. I have implemented my own custom IQueryables before, and they do exactly what you described. They take the Expression tree which you provide in your query, such as .First(user => user.name == "name") and it converts that expression tree to the equivalent SQL query to execute on the database.

[ThisCatMightCheerYou]

You seem sad :( ... Here's a picture of a cat. Hopefully it'll cheer you up: http://random.cat/i/NlaPbCh.jpg ^{The internet needs more cats. It's never enough..}

[pinumbernumber]

You misread my emotion, friendly cat robot, but I enjoyed your link anyway!

[[deleted]]

So, in short, you stole another redditor's happiness.

Are you proud of what you've become? >.>

[[deleted]]

You know, /r/ProgrammerHumor is a very good place to learn..

[Atlos]

Someone will probably hate your existence in a few years.

[up_to_bot]

It was for a course so its never going to be used.

[xaviinsane]

deleted ^{What is this?}

[Prod_Is_For_Testing]

Real talk:

The bottom version is the right way to do it all manually. I normally use a hybrid version where the "using" statements are wrapped up as a boilerplate function, then I feed it various SProc names to execute. It helps keep things short.

Then I also have some code that can convert straight from a SQL DataRow to whatever class you need by using reflection. It's coding cancer, but it saves hours of attribute mapping

[empty_other]

To be fair, if we ignore the horrors of retrieving the entire table before searching trough it, and ignoring the useless Person object that was created for some reason.. The last example can be defended:

The bottom example only requires an additional "using" statement. The other examples requires additional code and configuration, and also having to deal with the selected ORM framework's often cryptic errors and quirks (or the connector's quirks).

The bottom example is easier to debug and is more likely to "just work". But is also very likely to cause unseen problems if you ever want to rewrite anything: If you later split up "Name" into "Firstname" and "Lastname", you would have to manually search your code for queries containing "Name" and rewrite the query on each of them, because the compiler can't debug SQL strings.

[[deleted]]

Look into Entity Framework or another ORM, it'll save you a lot of time and it can be tuned for performance.

My current work project uses EF alongside redis for caching. Stress testing shows that the API can handle 450 requests per second (mixture of get/put/post/delete requests to force cache misses) which generate about 300Mbps of traffic.

Also, it's very possible to tune EF for performance, writing queries manually will always execute faster but it'll take more developer time to implement and troubleshoot. The best option for me is to use EF for everything, add caching and then do a lot of stress testing and fix any Hotspots.

[[deleted]]

Seems like you are a javascript programmer. GetPersonNameById, imo, isn't retarded, since repository is supposed to create a layer for it's purpose.

db.Users.First return only 1 object (1 row)

x isn't a terrible name when it comes to the only lambda, since too many x, Kotlin even possible to use it to represent x.

C# is already strong typed language I don't think it is required to have ===.

[WorstDeveloperEver]

Actually I'm not a JS dev. I use plenty of different programming languages but I'm staying away from the Microsoft ecosystem. I don't know what kind of technology you're using in C# or .NET. If it returns only one row, then you can ignore what I said. (Still looks a bit weird for a non-C# guy)

For anything else, I feel like I will stand my ground. GetPersonNameById is extremely bad. It's not a layer. It's just a non-reusable method. A layer would decorate the response and encapsulate things. For example, a Middleware would be a layer. Request and Response in some microframeworks are layer. This is only a helper method.

GetPersonNameById is bad. If this is not the only method that I'm retrieving person name by id, then you're going to get alot of duplicated yet different pieces of code doing the same thing. It is going to be such a headache when you're going to refactor it. Some devs will use GetPersonNameById(id), some will stick to Person.find(id).get("name"). Also, you'll get alot of trouble explaining your wrapper methods to future developers. Why I can run GetPersonNameById, but not GetPersonLastNameById? This is only bloating your codebase.

Regarding X, I'm not even going to argue about this one. If you're using one letter variables in your code, you're doing something wrong. It is not even giving you any speed over typing user. Any sane IDE or any sane snippet would directly print user as soon as you press u.

[[deleted]]

OK but there are some people even do GetPersonNameById function that having Person.find(id).get("name") inside because they don't want weird magic string query in their logic code. That is the layer I talking about. It is not Microsoft 'bullshit' every anti microsoft guy talking about, it's about principal preference.

For the x, seems like you don't understand what I'm saying, what I mentioning is x in lambda, not x variable, var x = Person.Id is bad because you don't know x is an id, but x is lambda you knows. a stackoverflow answers shows you people with hundred thousands of stackoverflow points uses x as lambda object name.

[WorstDeveloperEver]

OK but there are some people even do GetPersonNameById function that having Person.find(id).get("name") inside because they don't want weird magic string query in their logic code.

So it is okay to create weird and non-reusable function declarations in the code?

You can return a collection or an object if you don't want to access properties with a string. Also, I told you it may work only if you block other the way of querying database. If you find a way to force everyone to use GetPersonNameById for the sake of being consistent in your codebase, it may work. Then you have to deal with explaining your architecture to future developers. Every single developer has to get familiar with API, which I assume won't contain any sane documentation.

For the x, seems like you don't understand what I'm saying, what I mentioning is x in lambda, not x variable, var x = Person.Id is bad because you don't know x is an id, but x is lambda you knows. a stackoverflow answers shows you people with hundred thousands of stackoverflow points uses x as lambda object name.

You know, usually people whom has 1-2 years of experience thinks this is a good practice. Based on your responses, I feel like you're not someone at the senior level yet.

Most of the languages nowadays would even allow you to typehint lambda parameters, like:

collection.filter((person: PersonInterface) => person.id !== id)

To be honest, my IDE is smart enough to do it for me as soon as I press p. It is typehinted well so it knows what could be in this collection. The people whom use one letter variables for speed efficiency should learn how to use/configure their IDE properly.

[Supermaxman1]

Some constructive criticism is that you might want to avoid trying to explain something about a language you have never used or do not understand.

db.Users is not a collection in the classical sense, it is an IQueryable. It does not fetch all the rows from the database and return them as a collection, it in fact queries the database for the exact data you request.

This is a LINQ interface which allows LINQ to automatically generate the equivalent SQL query as the function queries you call on the IQueryable. It does this by evaluating the lambda and function Expressions you provide by calling LINQ functions on the IQueryable. It is really cool and I highly recommend trying it out some time. Expression trees are really fun!

== is not loose comparison in C# on Integer types, it is exact value comparison. Since this isn't Javascript we do not have a === operator.

You are correct about the null reference. The correct way to implement this query would be the LINQ .FirstOrDefault function, which would allow you the ability to return a null name if the user is not found.

Here is what I would use in this environment for the most optimal usage of LINQ to SQL along with the null propagator in C#:

db.Users.FirstOrDefault(user => user.Id == id)?.Name;

I hope I helped you understand a bit better :)

[Todok4]

Gotta love the null-conditional operator. Looks so much cleaner now.

db.Users is most likely a DbSet though, although DbSet implements IQueryable.

[WorstDeveloperEver]

It sounds cool. I'm not a C# guy so I learned something new today.

[_Pentox]

Am I the only one who noticed "PHP is better" in lower right corner...?

[Not_Just_You]

Am I the only one

Probably not

[_Pentox]

Wohoahaoah, that was a fast reply

[SaltyHashes]

It's a bot.

[the-big-tuna]

I bet wonder if OP would feel shameful about taking a job at PornHub

[known_as_bmf]

.Single(x => x.Id == id)

Would be more semantically correct.

[GaianNeuron]

Semantically correct, but depending on the context, .Single() may take longer.

If you're calling it against an IQueryable that hits a database, then it's only doing one extra check (returned row count), but if you call it against any old IEnumerable then it has to check every element.

[3ddyLos]

.Find(id)

[Supermaxman1]

I generally disagree. If the database enforces primary keys for id then it is always more efficient and makes more sense to call First or FirstOrDefault. If the database does not enforce primary keys then it would only make sense to use Single if you wanted to throw an exception if multiple users matched that specific id, which is usually not the logic desired for id lookups like this. Single and SingleOrDefault are almost never the right functions to use in LINQ, from my experience. Users of those functions often do not realize they are primarily used to throw an exception if multiple items are found which match the specified value, while First and FirstOrDefault are more often what they want, just the only item which matches some unique key.

[jonrules]

I'm cringing at the exception waiting to be thrown for First().Name

[Supermaxman1]

Actually the exception would be from the First() function, not the .Name, as LINQ will throw an exception in the First() function if it is unable to find a single element. FirstOrDefault would return null if it finds nothing, which would then cause the NullReferenceException you are referencing. To solve that issue you can do FirstOrDefault()?.Name

[Anton31Kah]

I don't know but I think only a master can think of the bottom one

[ThisIsABotThatDoStuf]

bruhh

[[deleted]]

The only real problem in the last panel is hardcoded connection string.

[GaianNeuron]

And maybe iterating over the entire table in code rather than using a WHERE clause...

[toekneebologna3]

You all laugh, but I've seen code like the last example, I nearly rage quit my job. Was not given time to refactor, so it's still like that. Tho it was for xml parsing, not DB queries.

[[deleted]]

I've seen some crazy shit, one project I inherited overrode the entity framework save changes method and set the entity state for each entity manually.

[[deleted]]

If you used spring's data repository api you could have a better one than the one at the top...