Using viewmodel create comper and edit

I have this really long httppost action results that I add and update 3 db tables I think it's too long and can get shorter and smarter.

```
public class AllVm
{
public AllVm()
{
this.wri = new Writer();
this.sub = new Subject();
this.rev = new Review();

}
public Writer wri { get; set; }
public Subject sub { get; set; }
public Review rev { get; set; }

}
}

public ActionResult Review()
{
return View();
}
[HttpPost]
public async Task Review(AllVm model)
{
var logedInUser = User.Identity.GetUserId();
var memberDetails = (from m in db.member.Where(a => a.ApplicationUserId == logedInUser) select m).FirstOrDefault();
//check if subject name allready exist
var reviewSubject = from c in db.Subject select c.SubjectName.ToString().ToLower();
var match = await reviewSubject.FirstOrDefaultAsync(stringToCheck => stringToCheck.Equals(model.sub.SubjectName.ToLower()));

model.rev.Created = DateTime.Now;
if (ModelState.IsValid)
{
if ((model.sub.SubjectName.ToLower()).Equals(match))

{
var ttt = (from c in db.Subject.Where(aa => aa.SubjectName.ToString().ToLower().Equals(match)) select c);
var GetSubjectId = await (from a in ttt select a.SubjectId).SingleAsync();

if (model.rev.GBU == "Good")
{
var Good = await (from a in ttt select a.Good).SingleAsync();
var bad = await (from a in ttt select a.bad).SingleAsync();
var Ugly = await (from a in ttt select a.Ugly).SingleAsync();
model.sub.bad = bad;
model.sub.Ugly = Ugly;
int iGoodRating = Convert.ToInt32(Good);
iGoodRating++;
model.sub.Good = iGoodRating;
model.sub.SubjectId = GetSubjectId;
db.Entry(mo

• Nested If-Statements: Nesting If-statements tends to lead to code that's harder to follow at times. So when you're able to reduce the amount of nesting that you do by returning early (once again this isn't always possible but it can be helpful to reduce nesting and therefore improve readability).

Since you do nothing after the ModelState.IsValid if-statement you could return early instead by flipping your condition like so:

if (!ModelState.IsValid) 
{ 
    return View(model);
}
// Now everything that was in your if-statement can go here just as it was but indented slightly less.

-
Naming & Consistency: Let's start with ttt it took me a minute (two) to figure out where this was being declared. Why did I need to know where it was being declared? Because I didn't know what it was. Name your variables something that describes them. Generally the more descriptive the better. Secondly, be consistent. You have a.Ugly, a.bad, and a.Good. Fix this by making it a.Bad a simple Right-Click -> Refactor -> Rename should make this a breeze.

-
Modularity: Make use of functions to make code more reusable and to improve readability. The contents of your if (model.rev.GBU == "Good") if-statement chain is all repeated code. Break that down into a function and just call that function 1-3 times (depending how you break it down). This will not only reduce the amount of code you have but it should make it a bit clearer on what you're doing (to others, you, or you 2 weeks from now).

-
ViewModel: Your view model has a few things wrong with it. The Name doesn't describe at all what it is for, I've suggested a probably inaccurate name to try and get my point across. Public class members should be PascalCased, and you should avoid truncating a variable name just to make it shorter. Now this being said, I have been told before that making a variable name the same as the class it represents is bad, but I have yet to have a problem doing so, you're welcome to add more to the member names but I wouldn't recommend truncating Writer to wri, Subject to sub, etc.

Remember: Right-Click -> Refactor -> Rename

public class WrittenPieceReviewVM
    {
        public Writer Writer { get; set; }
        public Subject Subject { get; set; }
        public Review Review { get; set; }

        public WrittenPieceReviewVM()
        {
            this.Writer = new Writer();
            this.Subject = new Subject();
            this.Review = new Review();
        }
    }

I hope these suggestions have helped you in some way.

StackExchange Code Review Q#119382, answer score: 5