5
\$\begingroup\$

I have a question concerning the correctness of the reading and displaying the image in ASP.NET Core MVC web application.

This is the way I am reading image names from a specific folder (I am using IHostingEnvironment _hostingEnvironment to get the rootPath):

 public class GetRandomImageForGalleryView : IGetRandomImageFromFolder
 {
 private string rootPath;
 public GetRandomImageForGalleryView(string rootPath)
 {
 this.rootPath = rootPath;
 }
 public string[] getImage()
 {
 return ReadPhotosFromDirectory();
 }
 private string[] ReadPhotosFromDirectory()
 {
 string[] fileLocations = Directory.GetFiles(rootPath+"\\lib\\Images\\Nature").Select(path => Path.GetFileName(path))
 .ToArray();
 return fileLocations;
 }
 }

And this is the way I am displaying them:

@model IGetRandomImageFromFolder
@{ 
 ViewBag.Title = "Gallery";
}
<div class="container">
 <div class="row">
 @{
 foreach (var item in Model.getImage())
 { 
 <img src="~/lib/Images/Nature/@item" style="max-width: 500px;">
 }
 }
 </div>
</div>

Basically I just replace the image name.

Is this a good way? If not, why? Any other remarks would be helpful as well.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jul 1, 2016 at 14:30
\$\endgroup\$
1
  • \$\begingroup\$ I would simplify the GetImage() method as suggested by @svick and I would also avoid the method call from the Razor view, build the list of files in a controller and then pass it to the view rather. \$\endgroup\$ Commented Jul 2, 2016 at 17:26

1 Answer 1

5
\$\begingroup\$

You could simplify your getImage() method pretty significantly to just:

public IEnumerable<string> GetImage() =>
 Directory.GetFiles(rootPath + @"\lib\Images\Nature").Select(Path.GetFileName);

Things I changed:

  1. Changed capitalization to follow .Net naming guidelines.
  2. Removed unnecessary method ReadPhotosFromDirectory().
  3. Removed unnecessary variable fileLocations.
  4. Used verbatim string to avoid having to escape backslashes.
  5. Used method group to delegate conversion instead of lambda.
  6. Got rid of ToArray(), which didn't serve any purpose, and changed return type to IEnumerable<string>.
  7. Used C# 6.0 expression bodied method.
answered Jul 1, 2016 at 23:57
\$\endgroup\$
2
  • \$\begingroup\$ Wow, that did not answer my question if it is a good way to display this way by passing in the image like that in the razor view, however you gave me a nice lesson to research. Yeah I am coming from Java background so naming is something I still will have to look around in .Net as well bunch of things I will look now differently at, big thanks :)) \$\endgroup\$ Commented Jul 2, 2016 at 17:52
  • \$\begingroup\$ @vsarunov there is nothing wrong in using razor to display your images this way \$\endgroup\$ Commented Jul 3, 2016 at 1:11

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.