HttpClient - The Correct way to Use + Code Review

what's up YouTube in today's video we're going to be looking at HTTP client and the correct way to use HTTP client in your applications now the way I'm doing this is by way of a code review so I'm gonna go through the issues with the code I'll shoot the original code talk about the issues that I see with that so on the my just code review issues some of them are to do with HTTP client primarily most of them are and then show you a solution to these problems of how what is the best way to to do this all of the code that I present here will be available for you on my github I'll put a link in the description below what I'd like you to do is follow along so download the code right now and then follow along and when I say you pause the video at certain places you want to pause the video and then look at the actual code on your end for that specific class only so you can spin as much time as you need you can scroll up and down there's only three methods here so it's not like a massive class but you know take your time see if you can find some of the issues that I'm gonna talk about and then continue on with the video to see if you found or highlight some of those issues or all of those issues to start with alright so any time I think you should pause not only should you pause the source code on your machine so you can scroll up and down and examine it to the extent that you need to examine it to then come with out with the result or a comment or whatever right okay so let's get started so let me introduce to you the the solution itself before we move any further here so this is IDE what I call what's wrong with the school - this w w w TC he a she be client and there's the original code that was part of a code view that I was doing at one point and then the second project in the solution is the fixed code and the third is the service where these things are actually requiring your service or what I did is that build a very simple service it doesn't do much it just responds to thee the quest that the HTV clients are making in this these projects so you have a way to work with this stuff independently and if you want to rewrite or e factor the whole thing that service should help you make sure your can on the right track so it's not a full up full-blown service with just end points that function so that this solution is standalone usually will get the solution run any of the projects against that webserver and it should just work right so in the httpclient original project there is a class here that you want to look at which is the authentication token provider right you know on the two and this one the one so there are two classes one is if you look at the names atomy I'll just zoom in here see and Cena so I'm talking primarily about these two pluses did the same name there's a original that the one that was kind of what I got in the code review request this one has my comments in line so when you want to start to kind understand what the issues are look at the one the version one if you are not going into version 2 has then the to do items at every every point in the code that highlights all the issues I've got that gets a bit too confusing that there's a lot of issues this little code here and I'll show you the code now so this is the court I want you to look at the work order service auth token provider not the v2 but just the author compiler and it's got these three methods so if you that's it it's got these three mothers create work order which is this one in or get access token and logo and in order to create a work order you need to first get the access token you use that access token to create the work order and once you're done you log out Cyril sequence of your own so this is the original glass and the coordinate is provided so I'm going to show you initially just a great work order method here if you want now you can pause here to look at this method to get a sense of what may or may not be wrong with this method so okay I'll assume you want to move forward now from here I'll expand this method here take a look at this method here so I made a bit here so you can pause okay then this method here which is the get access token method and a video for you were you singing pause all right so I'm assuming you've looked at the source code at the different methods in this one class and no cheating and you've made notes of all the things that are wrong now the surprise of this thing is now a lot of this is to do with programming with intent so that's a whole series and off of this video the part of that series there are a lot of things that I talked about in that programming the intent of PWI as I call it and so it's not kind of coming out of thin air right this is how the people I work with most times are working like this I'm training them going through this process and I'll call that PWI thing if you remember the very first video it's a learning process not just me but the teams I work with the people that I work with they could be suggestions that you know they give me that I incorporated as part of PWI over the years it's just PWI so you might find some of these things to be like oh my gosh I don't think so kind of things and that that's fine right this is how I've been programming for many many years now and for me it's natural for me it's quite normal I also see the problems like this kind of mom jump out at me and I know from experience that people who are learning this stuff it takes them a while to remember the thousands of things that you need to remember from a PWI standpoint from a programming of the ten standpoint alright so but nonetheless the issues I'm gonna highlight to you are broadly speaking three categories one are like just code review things you know namings stuff like that the next is bugs these are your bugs these I don't know how you dice it there's about right so it's a problem and then the third is design issues so most times code reviews and design reviews are kind of going hand-in-hand depending on who you're working with if I'm working with the senior we've probably worked offline offline me not in the code but generally you have given a guidance or something and so we're on the same page when I get the code I'm doing a design review slash code review I so it's kind of both so okay so there are how many items how many issues do you think there are broadly speaking there are about let me see let me show you here so you'll if you look at the version to code I have a bunch of items I've listed out here so from a general PWI perspective there are 12 issues from a bugs perspective there are 6 bugs and from a design perspective that I am in issues now these are literally the one time that I see that that means that's to say that sometimes things are repeated in the same issue the same mistakes the same issues are repeated in the same code many times I've only listed once so if you look at the two do's here in this file and you can look at a battery and a task list here there are over 50 to deuce for this one code is the same coordinates now I just got cluttered if you will with all there to do this Soviet if I didn't want to show you this part is because it just gets too confusing to see but I'm gonna now go through these issues here one by one and I'm gonna try and explain to you some reasoning ideas that I might have sometimes maybe if it's if things are so k if it's not but it'll be nice to have kind of thing all right okay should be internal yes all my classes I interned there very few classes that need to be public right so you need to make those choices these classes truly need to be public accessible from our side this assembly and by this SME I mean if it's some other assembly you're not sharing everything like typically in business applications when you make your classes you're neither one sharing anything but you might become zoomy other library is no good third parties whatever but you're not the one sharing so don't share you know make an internal affinity share within your own assemblies you can share internals within your own assemblies but you don't need to make them public to make them accessible to anyone that has access to your dear house should be sealed sending all classes in my world start internal sealed in fact if I were to create a new class that mean [Music] no I can just hide you do it just so in this project I'm going to create a new class and I just say you know class one thing it starts off internal seal might not modified the visual studio item template as they call it to make that happen where anytime I see a new class from anywhere and Visual Studio is gonna say internal seal that's how it starts solved all classes start Eliza's into a Lucille it's going to delete that [Music] okay back here okay then from these are all PWI design if you can see here the traduced oh by the way something happened in Visual Studio I don't know at what point twenty nineteen sixteen point two three or something like the the you we have the ability to define our own tasks custom task items like traduce we can define also I do that I have different I got a PW I got a PW I let me show you that is yeah yeah sorry there normally this to do have added PW bug PWI let's just non-priority PWI bug as a high priority and Peter via design is a high priority the idea is that when you have these define as your custom tasks liske list tokens you could and start doing it with a to do what you really need can do is this and you can save PWI and that's it and it shows up in the task list here as an item but in very just tree that's not working for that 2019 that's not working it used to work in 2019 but at some point some version of free 90 stopped working so I put to do this you can see them say we went to your task list you'll see all these items here and you can navigate to each of them by clicking here or just f8 will cycle you through each item in your entire solution and that's what I normally do when I'm looking at issues but okay going back here we have this PW I designed another class named it's because this guy came up to me this is a gateway right it's allowing me assisting me is to giving me the ability or capability to talk to some servers that in this case is the work order service and so that's if something's helping me talk to that service then that's the gateway manatees in my systems that's the gateway so and of course and broader scheme of things here this project has gateways - it's not like a dozen so when I see this problem and what I said that's not the right name for this this needs to be called gateway something and not I not work order service auth token provider which is not it is more than that so that's that this is a design issue here now a lot of they a pw+ design so these top ones here are just normal people there are things in the bottom three over here are design nice to look at popping mother should not be virtual which is a design guidelines favorite design guideline I've been following it for years there is some sort of a security risk associated with public void shows public methods that are virtual and so they recommend not having public methods a virtual what I recommend is having a public method does not virtual with the name whatever name e1 and then have that method calling a protected abstract method with the same name but with the suffix of core I so do it that Y so this so that's one issue here but the second issue that don't there's no need for so one says you should not have public methods Roberto that one and then names this is I have these things in pw-sat Dorian read names or we don't expose our privates or I need to know basis these are all phrases I use in my team so that is that morning phrase is kind of like a name of a design pattern explains a whole lot of things as I just say need-to-know basis that means something to people haven't explained the whole thing over and again but here I am solely repeating metho sudden don't invent names mean something what it means is don't invent a name of an argument parameter or a local variable give it the name of the type itself camelcase but give it the name of the type there's no need to try and invent a name right now unless there's a collision or for whatever reason there you need that specific specificity then I would say you need to name other it's just no need for it like if it has a business meaning the thing should have already had a name they said there's a class that's a business the model then it has a proper name to start with you don't need to invent a name for it I devised the thing like an HTTP client well if you only have one in this class then this Khalid HTV plan and there's no need to invent a name for it so if you name things correctly class names and method names and such then the variables and arguments parameters they would automatically be named exactly what you want them to be because they all do you name everything else the way you want them to be nice to know he no need to invent another name if you find yourself each time you create an instance of some class giving it a different name then you potentially name the class incorrectly nice especially if it's your class you didn't change the name of the class I mean if you always consider to be something else in your local in your methods as local variables you give it a different name then maybe the classes didn't name correctly in assuming you might want to change it there so don't even names just means use the same names and this is a collision if you need to HCP clients one going to this one going to that you know endpoint or service you might want to call this basically as per that what is it what are they going to or what are they doing exactly but there's no collision of some sort this quality leave it warning on the way it is the normal type name so what I'm saying here is this guy corner selling then call this vocalist an evolve in many name hmm I think it's odd I mean video studio will actually assist you to tell you that the VAR given name these cubes gonna give you a hint let me see make sure you what I mean so that look it here so there's this thing so why fight it's just this if you work with Visual Studio and stop against it this is what I've been doing over the years as I find which the studio coming up with features and such I IRI alter - by the way a program to sort of fit in with what Visual Studio is doing so I'm not fighting I'm not going against the grain of people I'm going with the flow and you get more productive that way so you know that's what I do this is this I used to have you know change all kinds of settings in Visual Studio today I can get out of the box installed yesterday I'm happy it just works exactly the way I want because that's what I've gotten used to right so but this is the feature there and why not use it then but you would need to yeah I'll just undo it since I don't know it didn't modify this cord they get confused when you see there okay you the next one is need-to-know basis what that means is this thing this thing over here now it's not truly the case in this particular I want to kind of call it out as it as a thing because it's very very common the need-to-know basis says if the method is asking for some information it should ask for only the information it requires and that's it nothing more nothing more need-to-know basis conversely if you're sending a method an argument you should be concerned am i giving this thing too much information or you know too little or too little cause you complain but is am I giving you too much information and the metaphor I use for that is imagine you go into a store and you're talking to you're buying something in the store you want the counter to pay for it let's say is a hundred bucks so the storekeeper says the sound box do you take out your wallet and give the storekeeper your wallet or do you get them knotted box name that's the difference you are giving information to another method you should be concerned that that method is asking for too much information then conversely as a good design method design you should be designing methods that ask for exactly what they need and nothing more so the case of like a DTO here that's it the DTO idea or ten arguments and your it needs to username/password and base your I mean so you want to make sure that you're only sending this class this method the information it needs and nothing more so if this thing has more information than those sleep properties then you need to change that some people ask me so you're gonna create a whole a let's say this you know the sing needs the method needs they say you know seven properties of the main class that lets has twenty properties just as an example right so the order the class that were caller settings has twenty properties of then this method needs similar those are you gonna have certain arguments and you know I'm gonna create and define a new class that has just a certain arguments and then this method lasts for that new class and then in the call side I would have to map from A to B and send it in is that too much of work sure but it's way better than having to deal with this nonsense because what happens here is you don't know what all this method is using and you don't understand the implications from the call side like what could it be possibly doing with these twenty argument that these twenty properties right it's harder to test it's hard to manage is harder like ahead around things so keep it to the bare minimum I have immediately define other class just because you know one is what these certain arguments that almost got twenty properties that's fine it's a one-time thing I mean it's not a big deal and creating a classroom registry is on that big it is just do it that's what I've learned over years and of course in the future in c-sharp a name we get the data types data class I know what they call that regards in Delft used to have records so I guess Delfy still hanging in there and c-sharp yeah so records and those will make it easy even simpler to define plus it's just one line and with the construct arguments range that's it yeah so get used to that idea I think that's the best way to go about this thing all right then this one here says that this method is the only one that needs to be public whereas you got good access token that's public and you've got la Garde s public create Walker so all everything is public but really the way this thing works is and this what I mean by the design right so there's not like mr. yogorov you won't know that unless you know how it's being used in order to create a work order you need to log in or get access to open you need to create the work order and then you need Lara is part of creating of Okada is the sequence of steps that you require to do in order to create a work order that doesn't mean that those two other methods need to be public is the one which also means comes come another thing that comes to me with regards to the stuff is in my case I offer every public method of a class I'm striving to make that public method the orchestrator in order to create a work order you forced her to logout login let's cut it that's another comment that I'd get access to which we call login then you create the worker and then a lot of the sequence of steps which means the create work order should be doing orchestration so that anyone looking at that implementation that oh I see is - in order to create a work already do these three steps that's it I don't see the clutter of the code right as you want to just see the orchestration make it simple you know let me read the thing I said tell me a story that's how you tell me a story you go step step step that's it sorry I'm jumping all over the place but that's the nature of our code revision either you can't explain them in isolation right no need for chills to this public virtual good access token is in this name essentially a login so I think they should be called a log and even though you're getting back an access token you're redoing a login it's easier to understand you first to log in then you do a creative work order then you do a lot of name okay so that's the issues with this method they're actually aiming to lose inside as well so I forgot about that so these were just kind of the method level my well let's go inside here into these methods here okay so leave blank lines is I think the original walls such that there's no blank lines yes it becomes difficult to read and of course if you're using sonar and other things they'll bring you on this right away so if you're not using sonar I'll probably be making a video soonish on installing and using sonar on the local machine so that you can connect our user on your local machine to highlight some issues and stuff is pretty cool if you have the right tool set the problem with sonar is that some people can have overboard with the rule set and I don't necessarily with all the rules that people think they should apply but I think the default rules of sonar the boss are perfectly fine barring a few that I will turn off this because they don't I don't subscribe to that kind of thing it's a good thing to learn it was sonar lint if you don't use sonar lint on your own video studio you should be using sonar lint the the effects cop''-style cop sorry non-stop ethics cop code analyzes and Somaliland analyzes you can install those in your be the project wide or visual studio while use those tools decide which tools you want to turn on/off and start to sort of form the habits but yeah you won't see any issues with this course this one are in my mind sonar is a subset of my tools but the right rules if you were yeah I won't go to the details but I'll be making a video so you can use sonar on your local machine and kind of you know improve your code that way all right it's going back here let's forget the cord now so leave blank lines which is understandable don't invent names again this was you know this call client my kinda speak all HP clients rolling my name's you're creating and uses of a client each time this method is called see each timing called gate access token you're creating a new instance of HP client that's a big problem all right so now coming to htb client you cannot I should not be creating new instances of HTTP client every single time through the lifetime of your application every time a method is called so typically you would have a class that either cashes in instance of HTTP client or Internet core you can use the HP client factory it's a bit I think dumb necessarily convoluted but you can use that and have the factory do the caching right so basically what's happening with the factory in dotnet core is you identify I'm keeping simple yeah and the 5htp clients by name or by type you can define your own custom own types and then it maintains a cache of that I saw a you asked it each time it'll give you back the same instance or maybe it'll create a new instance depending what's going on at that moment in time as a result you're reusing the client HP client instance the key aspect is reusing the clay HP client instance whether you do it yourself we use the factory doesn't matter make sure you do it off pain okay so that's that and so what's happening here and across all these methods they're each creating their own instance that means just in order to create a work order you have 3 HP client instances being created yeah that's ridiculous that's just too much and they've not even being disposed which is the other problem not only creating multiple instances here I call it the bug here they're not being disposed so these are coming under the the bug thing they should be client I shouldn't create multiple of those and the fact that they're you're not calling disposing them so these are bugs now I forgot to mention the core actually works exactly how you expect with the minor issue and I want you guys to figure out the issue I'm not gonna fix it in the corn I want you guys to figure it out made of an I show you can see it and then comment on seeing it's a very small issue and maybe after I finish this metas thing I'll show you the function of the application so that you know those who want to download the code can see how this how hard work the application the solution and then continue on with the code review then base URL so what's happening here is every time you call some method that you're providing this the base URI that's part of the settings you know your work order settings and that's also not a good name mother this work all the same is not correct so what I'm suggesting here is that the HTTP client has a way to set the base URL in the HTTP client itself and then for every pulse to get whatever you're doing you just need to provide it with that part right this constant concatenation going on is not really required right so that's what I mean by that again this request this should be called what should be called it should be request message I find it useful that the variable name is the type name now some people might say that's Hungarian notation over that's not Hungarian notation but that's what I'm saying that's what I mean when I say don't even names it's got the type name HTV request message didn't call it that like how much time does it take to type in the HTTP request message once we after that is the ID will help you write so that's from a little suggesting there and then these are disposable also but there's no place that these are being disposed now some of these things are the repetitive issues right in every method is the same problem again and again so that's what I meant by the items here like when I say issue a client should not be clearly each time it's not a one time issue that I daddy showed us three times here I do fear not doing this thing that same issues occurring three or four times in this last year so the top level is just the kinds of issues and the traduz are highlighting that issues at the definite Institute if you all right then then we have so that's disposable but it's not being disposed HP client is disposable that's not being disposed and tournament names then I can leave a punchline here don't mean in Indian names again that is an H to be a response message so this is called rec this is called response to me even if you're not naming things correctly maybe even if you don't follow my sort of way of naming things which the doughnut read names at least be consistent right if one is called response didn't call to the request you can't call one rec in their own response that doesn't make sense to me names that you define but make sure there's consistency so everyone on the team is calling it the same thing and everywhere in the same class or the same system you're calling the same thing the same thing and that's why the rather than trying to figure out world I call it the last time in that project or in that class don't my names is easier I just call it what it is simple then surrendering of it went back and forth they're distracted okay so the thing is that that's missing in all of the elevated methods is configured about false not configured false as I mentioned over here is you need it if you're not in dotnet core internet core use it anyways it doesn't really matter and don't know called there is no real cell you know separate context so you don't have that issue therefore you don't have the need to keep saying you know configure about false but if it's a library negative net 22.1 standard library then make sure you put in dotnet core I sorry the configure weight falls after every acing method as well for library classes things my bad so that that this issues they're everywhere did we do an available method make here as well that's a big problem here in this debug as you see see these are called like some of them are bugs some of them are just design and some of them I just PWI alright so this is the feed of the I she's disappeared of the bug issue this is a PWI design issue and so on each time you do a send when you get back a response HP response message you need to ensure that that message it may say the status code on it if you don't check the status quo and just assume it's gonna work it's gonna blow up in your face right so that's not the way you want to program after every time you do a send of some sort posts get put push whatever you don't get back a response and look at the status code all the response and there's a method on the response HP response message called ensure success status code at least use that in the code I assure you we'll be using another custom method that does that but you at least use that much right and then do the kind of make sure okay before you move forward over here is just assuming everything's gonna be okay sighs feeding the string even though the string may not be what you want yeah I expect to come back here so the skin access token it does of course it open here just before I forget that if you get this thing over here in the creative of order method is calling get access token and it's calling it access this is what I mean by consistency if if you don't have an idea if you don't have a convention for naming things you're gonna just every single time in my dear alone thing yeah you call it access they call it token or they you call to access token be you you're not following a convention so I'm not saying you should follow my convention or my you know don't a man named saying nothing that's the simplest because you don't have to invent anything board is supposed to type what's the method calling is called gate access token what do you think I'm going to call the variable access token it says get access token now if I don't like to call something access token I will rename the method to something else and now whatever says get XYZ I'll call this XYZ having a consistency in which the entire team names thing is way better than each one not only coming up with their own names every single method but in every single project and every single person has their own idea that's a bit much so the simplest solution is instead of inventing names use the names that are there that's simple all right then we have from a design issue this so now this if you haven't seen by the way I'm gonna put a few links in the video here this is for the gateway I'll put a link up here but the levels of abstraction I'll put a link up here and if you haven't watched those videos please watch them because these this video is kind of bitching across these two topics if you were nicer that might help you in case you're not sure what I'm saying when I say love abstraction I'll explain that in that video and what does that all the response piece of the gate we have explained that as well so okay I'll give you briefly what I mean here so this is the create work order method I'm actually use this classes don't have the cover okay there is what amended this what good-looking as in order to create a work order there are three things logon get an access token back feed that record of providing the access token 3 Laura that's it 3 steps why all the clutter I can't understand what's going on here is just a whole lot of clutter and this is a public method so I would like to see the orchestration now if we call that Laura that would be cool right it's long so you log in with the access token right then this all of this here is create work order and then there's lure out so we were to refactor this like that then I would see proper levels of obsession what I mean by levels of abstraction is that this method is that at a certain level abstraction you don't see the HTTP kinds of things in here this method also is a separate level abstraction from this method this is also you don't see all the stuff here and then next thing you know you're doing all this stuff yeah that's going down the weeds with regards to a should be client and JSON converting sting content and media type 1ba it's just know a lot of teachers is much lower level of abstraction then this that this method is that when you see something like that and you're not seeing all those details so you will need to extract this into other method so that now you three do you see three steps you see let's say login create a border blah blah not gonna get all this that this is not correct here it's working but it's not going okay let's look at the sort of the fixed version of our gateway okay let me actually show you how this thing works so now you know the solution has a service Santa client and I know a lot of people don't know how to run it list the service before the clients in you know in case they didn't all you start to instances obvious to do and just run the service first and run the client and stuff and there may be beneficial of the search takes too much time to spin up you could have a separate instance that just stays running you're not developing that service they just using another way to you know bounce against that's one way to do it I'll show you another way here in video studio I'm going to the properties for the solution project you can say normally I think it says single or kind of currents fiction I forget what which one is the normal you can say multiple and pick the ones you want say yeah I'm saying I wanted to start their work or the service and I can also arrange the order of the sequence I mean these can be moved up and down so start this one first and then start the original one so he's gonna start the service and then start the original console so it's starting these the service part and that's gonna start this console on so in the console app you got this and we use this thing here this is just basic information here then we create a new instance and we call this and I've got a laptop you know says original work version and then the work already so let's run this and see the output right so now in case you wanna lose a veneer now this is seemingly the correct output in fact what happened here is you write your tests to include this output this output ice-t is incorrect even though it looks correct it's a subtle subtle issue and I'm gonna leave this bug in there and I think both the versions have it I want you to fix it and I'll provide the solution in here break and I'll say pause then I'll give you the solution and then you can fix it all right so the bug here is that these little little I mean double quotes here pose be there this is not the value of the entire value includes a double course and that's not correct nice the value suppose I have no double course around it and that's the subtle bug let's set this up to run the another product said this to start you can also have them both running so they both spin up so that's also doable you don't have to done one off and done on the application yeah they both have the the issue so I'm gonna leave it here as I said I want you to fix it see what it is if you already know what could be the problem you can put that in a comment or either ways you can Spall's it you can look at all the code yourselves and then tell me what it is okay now the fixed version of this gateway is in this solution now Pollock HTV client fixed and it's got a little extra for folder structure in here it's what I use in my system so this is just a portion the gateway is not the part of my system to the whole system the fourth a gateway typically but this is a certain photo structure follows have made that in this project as well and so the gateway will be found in under services this in this project here I needed me zoomin that's the fixed business services folder here expand that you can have multiple sources say in your applications and so I'm working with the work order service and then everything to do with the work order services in this folder nice with a gateway the don't worry about the sauce agent that that's put that won't be there the mapper those models that resource models are models that the sole secuence is Ananda your models these are models of the service so I keep them separate and I don't want them leaking into my system in fact there's a PDI issue in the other court original court that says he ki leaky accorded eeky eeky leaky or some people just call a leaky API call it an achy leaky it's also loosey-goosey right so that's the resource models and then the gateway so we won't look at this gateway that's the kind of a fixed up version of this so this called a what called a service kiddies that's a proper name is implementing a disposable because they should be client is cashed in this class and this HP client is and I disposable so therefore this one will need to be a disposed Ruby says what was using that last flood need to be a disposable that's the I disposable that it kind of that goes up like a virus hmm not a good idea to talk about viruses and software these days but that's what I see it a sink kind of comes down and I disposal goes up they both canna the entire call chain gets altered okay so I won't go into details please examine this cord properly I just highlight some other things besides the fact that I've fixed things the constructor gets the work order settings right as opposed to if you look at the if you look at this yes there is no the Walker class method only guess the settings then the access token gets the sayings again in the log and lavato finally doesn't need it I'm saying that design should not be the case so only the settings are required by the gateways constructor one-time without that settings the gateway is not working so that at the construction you can give it that settings and it's not called what color settings because what Karla sayings is not quite what Karla setting is that the settings to create the work order or is it settings for the service right so if you look here it's called work order service settings so some subtle name changes have also occurred in the the solution parts is the fixed project called fixed over the sufferings of fixes the sort of the fixed version of the thing and in the construction constructor I'll create an instance of the HB client and when I created an instance of the HTTP client I said the base URI to the URI that comes from the settings so this HP client instance is always going to be going to that service therefore the base URI set to that end point all that services you are all and then every method can add on its own time on the ends of the URL but the base you aren't allowed to be he repeated over and over once you set up base URL you always assume there's that prefix if evil to your suffix of the URL all right so just take these things out of the way constructor and decorate that's not important and then I have a few methods here okay now cut this you can see right away there's many more methods but I have left this in this class because it's tonig let's say and you're starting out this is the only gateway in your system the moment I am more than one gateway you will refactor this into another class of some of these functionaries into another trance and I'll show you what that is all right but for now I leave it in one class one they don't add on ten or ten more classes right right out of the band this infant this capability is only required in this gateway so that's it I'll leave it here I'm happy with it I have no problem leaving things in a class if it's not required anywhere else I'm not a big fan of having a clutter of classes you know one class with two methods another class with three methods just because but that doesn't mean that I will leave it that way if I find that there are other classes that could use the same functionality right so I wait I don't do it right off the bat so in some cases they also do rather the van in case it's just too complicated the stuff it just needs to be out in a separate class so for now to keep it simple actually I've left in one class and I'll explain the create walk out of is of course this required in this class is the gateway sorry here it's create worker so these three will bind the constructor these three are the main methods that are required and of course the create workers only public method everything else is private and most of it is private static that's cool the others are and the censure success is prob'ly required as well inside this mas these other methods are sort of helper methods if you will in that they are dealing at a very low level that the HCV client with streams and media types and all that typically in a normal system I have many services therefore many gateways which means you abstract out the HTTP parts into a service agent again you might want to look at my the Gateway design pattern video here I talked about this over there the HTTP HTTP specific things are moved out I have turned around into a service agent I called a service agent HTTP and the Gateway parts which are more business see if you will to do with that business and the service and the business like Decatur is the mediator between your business system and that service but does the gate we have to be cluttered with all the advances of the protocol the HTTP part or can that be abstracted into a separate subcision and especially when you have multiple services then that commonality can be pushed out into another class called service agent and I have an example of that and this core base as well and you can take a look at that Nestor's or the v2 version of it so the v2 version of it uses the service agent that we've one version of it or the novel the first fixed version does not use the service agent but the second one does so you can look at how to do that in the source code I'll introduce it a little bit here but not going the detail of that so these methods could potentially be like actually abstracted or extracted from this class and these methods will be only ones in this class so I will show you the details of that but essentially what it's doing is it's doing the orchestration so at a cut-off here I can say oh look at that so first after map and this is the big thing with gateways right this is the model that is a part of my business the work order for create let's call well as the gateway requires another model called work order for create request the key key part of the original source code that I received which was here is that this create work order is asking the business side who's who's calling the Gateway something in the business side it's asking the business side to provide it with a walk or a create request that means is leaking the services models into your business system the gateways job is to abstract to you from that the gateways job will say is to work with your API your models and do the mapping to talk to the service other than the other way around so that's what that is so that business model and this you know walk order create request is the eServices moral right so that's the citizen was the gateway will map from that to that they may or may not be the same maybe property names are different fewer lesser more ordered that map will handle all of that then call the login then called the pose for the crate and then call the logger now you could argue that this portion also should be abstracted to other method discs we just called create record and I have done that in the version 2 of this class which I'll show you next but that's the basic ostraka stage and it's still a little cleaner than before and of course don't call it get access to open it's called login you don't show what this is doing exactly and then there's a logger it's you're not quite seen the orchestration here and then there's the other methods here the ensure success mat is a whole blown out method here in this class and it's doing a whole bunch of things and if you remember earlier I said that when we do talk to let's say if I look at the login method here after heavy send or post or whatever I call ensure success this is not the built-in ensure success on the HTV response message this is my own extension on extension just method in this gateway that asks for the bare minimum information and then based on that will determine what exception to throw I see mine won't look at this methods implementation this is very specific to this service it done actually this service I'm bill doesn't throw the arrows but this is what as I had received from for the code review and it's functional but luckily I don't have an endpoint to give you that this function will you see how how that they programmed we our own custom ensure success method that um this call after every post or get or push whatever and every every time we do a post in the post async here it's done inside this method itself and show success those saw the box this class is disposable so does the dispose and ensure their thing is disposed so we only have one instance of an H to be fined that is disposed and all these are disposable stays to respond message message is being disposed the sorry that's request message response message okay so look at this Gordon and you can see how that's been paid out how because you also want to make sure the core is clean and you know it's not like cluttered - barring the public method everything all the element should be readable as well I'm gonna show you the version two of the Gateway that uses this service agent so let me also issue either thing here so what I'm talking about is this is the fixed word and nothing wrong with it but the version 2 uses the source agent so if you want to look at the version 2 you'll see that it actually uses so they introduce all the HTTP parts of things this is not a requirement is just if you're using multiple sources in a system which is pretty common these days then rather than repeating the whole HTTP things and every gateway the best thing to do is and abstracts that HTV part also how I send stuff as JSON receive stuff as JSON send things as HTML to see with HTML plain text and so on that commonality can be put into service agent I call it sauce agent HDTV this is listen HTTP based subs and so they just could have any protocol and then the Gateway is just working with its bare minimum stop so let's together so the version too similar constructor nothing different there except now it's called a substation also and the source agent also I don't mean also there's no intro httpclient sauce agent in the sauce agent is also disposable because of the is to be client being disposable and so then in the dispose method you got to do that that's wrong and said disposable is like a virus going upstream work order destiny public method here it says login it work order internal bless method also called create pokhara and logout that's it and simpler cleaner these the methods in here don't even know there's an HTP client it doesn't know what the protocol is it doesn't know what HP client is it's simply saying this is how I want things done I'm going to package up the data in a certain way I expect the data coming back to me in a certain way and then the source agent that's dealing with purely HTTP things doesn't understand the business it only understands HTTP and so you can't get it to your business I think that's what the Gateway does this is a difficult line to draw between the gateway in this house agent because the Gateway is implying that I know what I need how to marry they bridge the gap between the business side and the server side but I don't know what the protocol parts of the service the service agencies and all about the protocol but I don't know anything about what you need from a business standpoint so you have to make sure each one is talking the language then they're supposed to understand this is not it it's not an easy line to draw but you'll see that you know and if you look at the service agent thing that's that so been a long video the source code is there hope you enjoy the source code as well but here's what we learned so far court views are tough right you gotta find the bugs you gotta find the maintainability issue as a code reviewer as a programmer you got to do your best to not have bugs not have maintainability issues as you know so responsibilities that both both people aboard parties share riding clean good cord is really hot but the benefits are thee the payback is huge right and once it becomes a habit it just feel normal you don't spend any more time writing claim code right don't go thinking this is just a prototype next you know that prototype code has landed itself in production right that's not what you want so pay attention form good habits if you don't know what I'm talking one of the guys to PWI our programming with the ten I'll put up my playlist up here please take a look at that because that it is key but I hope you enjoyed this video from the perspective that there are that cord looks very simple I mean you would have never thought that that had that cord had 50 or issues to it right maybe don't agree with some of them but you can't say there's nothing wrong there are bugs there are definitely maintainability issues and there are definitely design issues okay but you don't know that you could possibly imagine from a little cord piece of coal like that that are 50 or issues with it 30 places in which the ratio so that this is huge and so the code let me have a comments let me tell the dummy oh the bug the bug needs to be fixed now tell me what the fix for the bug is so if you want to kind of not hear that part then I shall say goodbye to you right now thank you for watching and I'll see you next time but I'm gonna go back and show those who want to look at the issue in case you've already solved it or want to know the answer okay here's the deal for the for the original project is it's easy to look at the original puzzle and find the problem it's a read the original source code here the basic issue is this sub-header what you're saying when you do that you're saying is to the service you're calling I accept JSON in this case the the the content-type is saying I'm sending you JSON but the accept header the thing I want or accept JSON so the service is responding with JSON right what you're doing here [Music] this is not arranged correctly discord that's why a little bit confused like what the heck what's it doing there and then doing a lot more than doing this is just ridiculous any read a string sir reading you this ring the services NDG sauces / double course on it and we're just seeing existing so this is a fix to that let me see that you know sure that fixed me make sure they this product is set up badly [Music] No okay Miran of chewy the problem once again and fixed okay so this is the problem the double courts they are incorrect the data is fine is this tough to drive the bed of course necessary so the fix to that would be to not read this as string or or two ways one is don't say I accept JSON where that doesn't make sense you're not so if you did that then you will find that the strings the double course disappear that's the correct that's the correct output you don't want the double quotes that's the fix the other way to fix it would be to then use the read as extension method read as async and give it the type and type of B string so I won't do that here but that's what the that's what the solution be I could show you here and this the new version so the same the new version also has got the same bug and I left that intentionally so let's don't start this and turn this off get on the new version here on the services the Gateway to this gateway here as God the mmm both tasting it so there this is doing a deed as string acing but if I didn't just read as a sink okay let's foresee that the issue is not going to do some magic it's and I'm gonna run this so you can see this new version also has the issue and you see the whole double course there so it and I were to fix this and the create water piece here says to walk or ID someone say read as a sink you know they sink and that I want to say is a string is not like a DJ or something solid and something I couldn't ascend that accept her of that because Jason is gonna come back to me as JSON below here I'm gonna so assuming it's a string which is the problem I would kind of convert this JSON to a string on this application here they're not fixes that as well right so there's no double cords here this is two fixes to it depending on what's going on in your application if the service is going to be a JSON service then of course is gonna be responding to an Jason that means you would also say except Jason if that service is not only a JSON service is gonna respond to you in some cases strings in some cases JSON then you know it depends what how the fixes does the fix is affixed to say I don't accept JSON or is the fix to say I want to read data as JSON and that's the difference alright that brings us to the end of this video I hope you've enjoyed yourselves I hope you learned a few things if you have please give me a thumbs up and I will see you next time

Channel: Shiv Kumar

Views: 6,314

Keywords: C#, .NET Core, ASP.NET Core, .NET, ASP.NET, HttpClient, REST, SOAP, Web Services, OOD, OOP, Programming with Intent, Code Reviews, Code Reviewer

Id: CtHcT5X_rYQ

Length: 57min 42sec (3462 seconds)

Published: Sat Jul 04 2020