current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

I have 2 code block to do same a work and have same target create some thing, but I don't know which block is the best (I mean LINQ or non-LINQ)?

First of all, can anyone tell me how to optimize on the code blocks? And the lastly, help me find the best style between LINQ and non-LINQ:

First block is:

if (tblone.Rows.Count > 0)
        {
            tblone.Rows.Cast<DataRow>().Where(x => (string)x["id"] == "true").ToList()
                .ForEach(x => tbltwo.Rows.Cast<DataRow>().Where(y => y["category"] == x["cate"]).ToList().ForEach(
                    z =>
                    {
                        tblthree.Rows.Cast<DataRow>()
                                          .ToList<DataRow>()
                                          .ForEach(k => datatemp.Rows.Add(k["m"].ToString()));
                        tblfour.Rows.Cast<DataRow>()
                            .ToList<DataRow>()
                            .ForEach(k => datatemp.Rows.Add(k["m"].ToString()));
                    }));
        }

Second block is:

if (tblone.Rows.Count > 0)
        {
            for (var idx = 0; idx < tblone.Rows.Count; idx++)
            {
                if (bool.Parse(tblone.Rows[idx]["subset"].ToString()))
                {
                    for (var kdx = 0; kdx < tbltwo.Rows.Count; kdx++)
                    {
                        if (tbltwo.Rows[kdx]["category"].ToString() == tblone.Rows[idx]["cate"].ToString())
                        {
                            for (var zdx = 0; zdx < tblthree.Rows.Count; zdx++)
                                datatemp.Rows.Add(tblthree.Rows[zdx]["m"].ToString());
                            for (var zdx = 0; zdx < tblfour.Rows.Count; zdx++)
                                datatemp.Rows.Add(tblfour.Rows[zdx]["m"].ToString());
                        }
                    }
                }
            }
        }
share|improve this question
3  
ForEach() is not LINQ. –  svick Aug 14 '14 at 11:04

1 Answer 1

up vote 4 down vote

First - if you are working with DataTables, then you can use Linq to DataSet, which has nice extension methods to get field values.

Now let's review logic of your code. You are not using any data from tblone and tbltwo in your result. Actually you only finding joined categories between these two tables, and for each matched rows pair you are adding all m values from tblthree and tblfour to result. I find this logic pretty strange. Maybe you are expecting only single match here. Then you will need boolean result (match found or not). But basically you need count of joined rows:

var matchedCategoriesCount = 
            (from one in tblone.AsEnumerable().Where(r => r.Field<bool>("id"))
             join two in tbltwo.AsEnumerable()
                on one.Field<string>("cate") equals two.Field<string>("category")
             select one).Count();

Same with lambda syntax if you find it more readable:

var matchedCategoriesCount = 
            tblone.AsEnumerable().Where(r => r.Field<bool>("id"))
                  .Join(tbltwo.AsEnumerable(),
                        one => one.Field<string>("cate"),
                        two => two.Field<string>("category"),
                        (one, two) => one)
                  .Count();

Then you can select m values from tblthree and tblfour N times and create new DataTable:

// if (matchedCategoriesCount > 0)
var values = tblthree.AsEnumerable().Concat(tblfour.AsEnumerable())
                     .Select(r => r.Field<string>("m")))
                     .ToArray();

You can even use simple loop here:

for(int i = 0; i < matchedCategoriesCount; i++)
   foreach(var value in values)
       datatemp.Rows.Add(value);

And last - don't use variable names like tblone, tbltwo, tblthree etc. These names does not explain what data these variables hold. Use names from domain of your application.

share|improve this answer
    
Thank you so much,but AsEnumerable().Select(something) . I can't find Select() method after AsEnumerable(). –  user51150 Aug 14 '14 at 9:55
    
And next problem is Join. My code mind that : find and loop each rows in table one and find the id field is true. After that, find in table two match value with category field and cate. So, Join is okie , why you suggested that only count ()?? I want to loop each rows in match condition ! Please explain more detail for me . –  user51150 Aug 14 '14 at 9:58
    
I found Linq to DataSet , But I can't find the namespace , please tell me the namespace :) –  user51150 Aug 14 '14 at 9:59
    
@user51150 you can find extensions in same namespaces as classes which they are extent - System.Data. Make sure you have link to System.Data.DataSetExtensions.dll –  Sergey Berezovskiy Aug 14 '14 at 10:02
1  
Oh, I totally understand, You are right , count() and loop value. And I find the namespace of LINQ to DataSet. I vote for you ! –  user51150 Aug 14 '14 at 10:02

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.