开发者

Code structure, a methods should only do one thing

I am having a bit following the "a method should only do one thing"

I have a car text file, and if it contains even one BMW I want to set isValid to true, but while I am going through the text file anyways I thought I would also populate two list high end models(M3,M5 etc) and lower model (335, X3 etc).

I know that method should only do one thing, but it seems so convenient for it to also populate the lists. Here is what I h开发者_运维百科ave:

private bool hasBMWegments()
{
   foreach (ClassLib.CarSegment carElement in CarSegmentFactory.ContainsCar("BMW"))
   {
     isValid = true;
     if (carElement.Class.IndexOfAny(lowerModels) == 0)
     {
        lstOlderSegment.Add(carElement.ElementNumber);
     }
     if (carElementClass.IndexOfAny(upperModels) == 0)
     {
        lstNewerSegment.Add(carElement.ElementNumber);
     }
   }
   return isValid;
 }

Should I just create a method that performs the foreach check again? Or should I create another method inside that method (I would think that would be messy, and wouldn't related to the method name)

edit: sorry working with framework 2.0


I find that code to be a mess compared to this:

private IEnumerable<ClassLib.CarSegment>
    GetModels(IEnumerable<ClassLib.CarSegment> segments, string modelID)
{
    return segments.Where(x => x.Class.IndexOfAny(modelID) == 0);
}

// ...

var bmwSegments = CarSegmentFactory.ContainsCar("BMW").ToArray();

bool isValid = bmwSegments.Any();
var olderModelSegments = GetModels(bmwSegments, lowerModels);
var newerModelSegments = GetModels(bmwSegments, upperModels);

This code is obviously correct at a glance. The other code makes you look twice at the loop to figure out what's going on.


It looks like all you're doing is setting isValid to true on the first pass through the foreach. So all isValid really means is "is there at least one element?".

In which case you do not need to iterate twice. You can use Any() to do the valid check:

bool IsValid(IEnumerable<CarSegment> elements)
{
    return elements.Any();
}

void PopulateSegments(IEnumerable<CarSegment> elements)
{
    foreach(var element in elements)
    {
        //add to lists
    }
}
0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜