0
\$\begingroup\$

In my asp.net web API 2, I am reading values from web.config from the controller into my DTO as a response to the client. It is working as expected, I wonder if there are any improvements to make it better. Should I read it from a database or elsewhere? So I would be glad if you can share your comments.

Here is the controller related part:

[HttpGet]
        [Route("reconciliation")]
        public async Task<IHttpActionResult> GameReconciliation(ReconciliationDto reconciliationDto)
        {
            if (!ModelState.IsValid) return BadRequest(ModelState);

            using var recon = await _gameServices.GameReconciliation(reconciliationDto);

            

            switch (recon.StatusCode)
            {
                case HttpStatusCode.NotFound:
                    {
                        return NotFound();
                    }
                case HttpStatusCode.InternalServerError:
                    {
                        return InternalServerError();
                    }
                case HttpStatusCode.OK:
                    {
                        var responseStream = await recon.Content.ReadAsStringAsync();

                        var resultResponse = JsonConvert.DeserializeObject<ReconciliationResponse>(responseStream);

                        //Transform ReconciliationResponse into DTO for returning result
                        var config = new MapperConfiguration(cfg =>
                        {
                            cfg.CreateMap<ReconciliationResponse, ReconciliationResponseDto>().ForMember(x => x.ReconDateTime,
                                opt => opt.MapFrom(src => (src.ReconciliationResponseDateTime.ToString("yyyy-MM-dd"))));
                        });
                        var iMapper = config.CreateMapper();
                        var resultDto = iMapper.Map<ReconciliationResponse, ReconciliationResponseDto>(resultResponse);
                        //EAN Barcode Added
                        **resultDto.Ean = WebConfigurationManager.AppSettings["000000001570"];**
                        return Ok(resultDto);
                    }
                case HttpStatusCode.Unauthorized:
                    {
                        return Unauthorized();
                    }
                case HttpStatusCode.RequestTimeout:
                    {
                        return InternalServerError();
                    }
            }

            recon.Dispose();

            return Ok(recon);
        }

Here is web.config related part:

 <appSettings>
    <!--TEST EAN-->
    <add key="000000001570" value="9799753293685" />
\$\endgroup\$
0
0
\$\begingroup\$

Honestly, it is impossible for us to answer your question.

  • If this is the same value on all environments and it will likely never change, why not hardcode it in the C# code as a const in some appropriately named class?
  • If it changes per environment: put it in the .config.
  • If this is a value that might change regularly and you don't want to do a deploy each time: put it in the DB.

WRT your code:

  • Keep controllers and the methods therein small. Use something like MediatR to move logic to specific classes.

  • Don't needlessly abbreviate. Sure, the next maintainer of this code can figure out what recon means and/or what it is, but they shouldn't need to spend time on this: your code should make it clear what it is/does.

  • Don't use meaningless prefixes. I guess iMapper is an IMapper, but that doesn't mean you need to call it that. Plus, I assume CreateMapper returns an actual mapper, not simply an interface.

  • Why is your mapping logic inside this class, even inside this method? I'd expect that kind of logic to be in a separate class, and that class to be injected. For instance via your API's Startup class by using the AddAutoMapper method (if you're using AutoMapper).

  • I'm not a fan of seeing things like WebConfigurationManager.AppSettings["000000001570"]; in the middle of code. I usually put all the configuration settings in a specific static class (named e.g. "WebApiConfiguration") and all of the settings would be in that class as public static properties, e.g. public static string EnvironmentName => ConfigurationManager.AppSettings["EnvironmentName"];. This also allows me to have typed settings if necessary, e.g. public static bool LogDetails => _lazyLogDetails.Value; uses private static readonly Lazy<bool> _lazyLogDetails = new Lazy<bool>(() => ConfigurationHelper.GetBool("LogDetails")); which itself uses a method from this class.

\$\endgroup\$

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.