2
\$\begingroup\$

A little look on what are Dictionaries:

dictWordCounter<string, int> 
dictWordPercent<string, double>
topTwentySeven<string, double>
  • WordCounter is the number of words. For example, if the word frog is 10 times it will be {frog, 10}.

  • WordPercent is the percent after some math in the program. For example, frog could be {frog, 0,55}.

  • topTwentySeven is the top 27 values from the WordPercent dictionary that I want to operate on.

So, I need to count something like this:

  • If a word is just one, count it normally
  • If a word is twice or more, count it x2
  • Total number of words (after x2) must be still 27, not more

For example if it was "topThree" and words:

{Frog, 2}, {Monkey, 1}, {Giraffee, 1}, and their values from Percent Dictionary would be {Frog, 0,1}, {Giraffee, 0,2}, {Monkey, 0,45}, I want to count: 0,1*0,1*0,2 and (1-0,1) * (1-0,1)*(1-0,2).

Could someone look on this and check if it can be improved?

   var topTwentySeven = dictWordPercent.OrderByDescending(kvp => Math.Abs(kvp.Value - 0.5))
        .Take(27)
        .ToDictionary(kvp => kvp.Key, kvp => kvp.Value);

    int compare = 0;
    double temp2 = 1.0, A = 1.0, B = 1.0;
    for (int counter = 0; counter < 27; )
    {               

        foreach (var word in topTwentySeven)
        {
        dictWordCounter.TryGetValue(word.Key, out compare);
            if (compare >= 2 && counter < 26)
            {
                temp = word.Value;
                temp2 = temp * temp;
                A *= temp2;
                B *= (1 - temp2);
                counter = counter + 2;

            }
            else if (compare == 1 && counter < 27)
            {
                temp = word.Value;
                A *= temp;
                B *= (1 - temp);
                counter = counter + 1;
            }
            else if (counter >= 27)
            {
                //Do nothing
            }
        }
    }

int top27C = topTwentySeven.Count();

double aN = 0.0, bN = 0.0;
aN = Math.Pow(A, (1 / top27C));
bN = Math.Pow(B, (1 / top27C));

I need both A and B.

totalSum = Math.Round(((aN / (aN + bN)) * 100), 2);
\$\endgroup\$
  • \$\begingroup\$ I knew there was a reason to use . as decimal point \$\endgroup\$ – Ewan May 10 '15 at 11:56
  • \$\begingroup\$ Where's the method signature? Can you include that please? \$\endgroup\$ – RubberDuck May 10 '15 at 11:59
  • \$\begingroup\$ Which method? The one converting WordCounter to WordPercentage? \$\endgroup\$ – Ken'ichi Matsuyama May 10 '15 at 12:01
  • \$\begingroup\$ i'm confused. you say count twice but then square the value? You should be able to reduce this down to a single line of linq. But I dont understand the requirement \$\endgroup\$ – Ewan May 10 '15 at 12:01
  • \$\begingroup\$ @Ewan twice in the meaning of square, I am multiplying ( * ) all the values from dictionary. So if all the values would be single in WordCount I would multiple all of them, but if one of the values would be 2,3,4..n times in the wordCount dictionary I would multiply by this value twice, but keep in mind to count only 27 total values. \$\endgroup\$ – Ken'ichi Matsuyama May 10 '15 at 12:43
2
\$\begingroup\$

Updated. You have some tricky bits to do in linq, but you can see the code is much clearer (although less efficient) without the loop. You could further neaten, by creating a class WordCountAndValue with methods rather than using the anon type and inline ifs

using System;
using System.Linq;
using System.Collections.Generic;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace UnitTestProject3
{
    [TestClass]
    public class UnitTest1
    {
        [TestMethod]
        public void TestMethod1()
        {
            Dictionary<string, int> wordCounts = new Dictionary<string, int>();
            wordCounts.Add("t1", 1);
            wordCounts.Add("t2", 2);
            wordCounts.Add("t3", 3);
            wordCounts.Add("t4", 4);
            wordCounts.Add("t5", 5);

            Dictionary<string, double> wordPercent = new Dictionary<string, double>();
            wordPercent.Add("t1", 0.3);
            wordPercent.Add("t2", 0.4);
            wordPercent.Add("t3", 0.5);
            wordPercent.Add("t4", 0.6);
            wordPercent.Add("t5", 0.7);

            var wordsCombined = wordPercent
                .Select(kvp => new { word = kvp.Key, percent = kvp.Value, count = GetCount(wordCounts, kvp.Key) })
                .OrderByDescending(i => i.percent);

            //find number to get
            int target = 5;
            int count = 0;
            var sample = wordsCombined.TakeWhile(i=> target > (count += (i.count >= 2 ? 2 : i.count))).ToList();

            Assert.AreEqual(2, sample.Count());

            //find A
            double A = sample
                .Select(i => i.count >= 2 ? i.percent * i.percent : i.percent)
                .Aggregate(1.0, (t, n) => t * n);

            //find B
            double B = sample
                .Select(i => i.count >= 2 ? i.percent * i.percent : i.percent)
                .Aggregate(1.0, (t, n) => t * (1-n));

            Assert.AreEqual(2, sample.Count());
            Assert.AreEqual((double)0.1764, Math.Round(A,4), "A is incorrect");
            Assert.AreEqual((double)0.3264, Math.Round(B, 4), "B is incorrect");

        }

        public int GetCount(Dictionary<string, int> counts, string key)
        {
            if (!counts.ContainsKey(key))
            {
                return 0;
            }
            return counts[key];
        }

    }
}
\$\endgroup\$
2
\$\begingroup\$

Don't write dead code - it rots like a corpse and makes it hard to maintain your code:

else if (counter >= 27)
{
    //Do nothing
}

Just delete this.


You can use the ++ increment operator here:

counter = counter + 1;

Like this:

++counter;

There is a += operator you can use here:

counter = counter + 2;

Like this:

counter += 2;

There is a Math.Pow() method you can use:

temp2 = temp * temp;

Like this:

temp2 = Math.Pow(temp, 2);
\$\endgroup\$
  • \$\begingroup\$ I would have written counter++. It's a little easier to read and you don't care if it's incremented before or after the value is accessed. \$\endgroup\$ – d347hm4n May 11 '15 at 7:39

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.