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 2 down vote favorite

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);
share|improve this question
    
I knew there was a reason to use . as decimal point –  Ewan May 10 at 11:56
    
Where's the method signature? Can you include that please? –  RubberDuck May 10 at 11:59
    
Which method? The one converting WordCounter to WordPercentage? –  Ken'ichi Matsuyama May 10 at 12:01
    
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 –  Ewan May 10 at 12:01
    
@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. –  Ken'ichi Matsuyama May 10 at 12:43

2 Answers 2

up vote 2 down vote accepted

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];
        }

    }
}
share|improve this answer
    
I'll try it, thanks ;) –  Ken'ichi Matsuyama May 11 at 18:38
up vote 2 down vote

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);
share|improve this answer
    
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. –  d347hm4n May 11 at 7:39

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.