问卷设计难题

Question

我正在设计一个小的问卷调查web服务应用程序，但我想如果有人能看我的设计，看看它是否好的方法，我正在采取。

应用程序有以下类：

• Questionnaire
• Answer
• IQuestionRepository
• QuestionRepository implements IQuestionRepository

在QuestionRepository中，我有三个方法：

• GetQuestionnaire()将问卷列表返回给客户
• GetAnswerOptions(int questionId)在创建问题单对象时，在调查问卷构造函数中调用该方法，以获得每个问题的可能答案列表。这是我不确定要在问题单构造函数中创建存储库对象的部分，因为我在考虑将可能的答案列表插入到问题单对象的最佳方法是什么。
• GetMarks标记问卷结果

    [DataContract]
    public class Questionnaire
    {


        public Questionnaire() {
        }


        public Questionnaire(int _questionId) {
            QuestionId = _questionId;
            PossibleAnswer = new QuestionRepository().GetAnswerOptions(_questionId);
        }

        [DataMember]
        public int QuestionId { get; set; }

        [DataMember]
        public string QuestionAsk { get; set; }

        [DataMember]
        public string Title { get; set; }

        [DataMember]
        public string UserAnsResponse { get; set; }

        [DataMember]
        public string PossibleAnswer { get; set; }

        [DataMember]
        public int QuestionnaireType { get; set; }

        [DataMember]
        public bool? isCorrectAnswer { get; set; }

    }
    [DataContract]
    public class Answer {
        [DataMember]
        public int AnswerId { get; set; }
        [DataMember]
        public string AnswerText { get; set; }
        [DataMember]
        public Questionnaire Question { get; set; }
    }
    public interface IQuestionRepository
    {
        List<Questionnaire> GetQuestionnaire();

        double GetMarks(List<Questionnaire> qList);

        string GetAnswerOptions(int questionId);
    }
public class QuestionRepository : IQuestionRepository
{

    //this is a hack normally will pull data from a data source like DB/CSV
    public static List<Answer> GetAnswerLibrary() {

        List<Answer> AnsLibrary = new List<Answer>();

        AnsLibrary.Add(new Answer() { AnswerId = 1, AnswerText = "London", Question = new Questionnaire() { QuestionId= 11 } });
        AnsLibrary.Add(new Answer() { AnswerId = 2, AnswerText = "paris", Question = new Questionnaire() { QuestionId = 22 } });
        AnsLibrary.Add(new Answer() { AnswerId = 3, AnswerText = "Warsaw", Question = new Questionnaire() { QuestionId = 33 } });
        AnsLibrary.Add(new Answer() { AnswerId = 4, AnswerText = "Port louis", Question = new Questionnaire() { QuestionId = 44 } });
        AnsLibrary.Add(new Answer() { AnswerId = 5, AnswerText = "Berlin", Question = new Questionnaire() { QuestionId = 55 } });

        return AnsLibrary;
    }
    //this is a hack normally will pull data from a data source like DB/CSV
    public static List<Questionnaire> GetQuestionLibrary() {
        List<Questionnaire> QuestionLibrary = new List<Questionnaire>();

        QuestionLibrary.Add(new Questionnaire(11) { Title = "Geo", QuestionAsk = "What is the capital of England", QuestionnaireType = 1 });
        QuestionLibrary.Add(new Questionnaire(22) { Title = "Geo", QuestionAsk = "What is the capital of France", QuestionnaireType = 1 });
        QuestionLibrary.Add(new Questionnaire(33) { Title = "Geo", QuestionAsk = "What is the capital of Poland", QuestionnaireType = 1 });
        QuestionLibrary.Add(new Questionnaire(44) { Title = "Geo", QuestionAsk = "What is the capital of Mauritius", QuestionnaireType = 1 });
        QuestionLibrary.Add(new Questionnaire(55) { Title = "Geo", QuestionAsk = "What is the capital of Germany", QuestionnaireType = 1 });

        return QuestionLibrary;
    }
    public List<Questionnaire> GetQuestionnaire()
    {
        return GetQuestionLibrary();
    }

    public string GetAnswerOptions(int questionId) {

        string ans = string.Empty;

        var Anslibrary = GetAnswerLibrary();
        Dictionary<int, string> GetPossibleAnswerCombination = new Dictionary<int, string>(); 
        if (Anslibrary.Count > 0){
            try {

                bool CheckIsAnswerExisr = Anslibrary.Any(x => x.Question.QuestionId == questionId);
                if (CheckIsAnswerExisr) {

                    var ValidAnswer = Anslibrary.Where(x => x.Question.QuestionId == questionId).FirstOrDefault();
                    GetPossibleAnswerCombination.Add(ValidAnswer.AnswerId, ValidAnswer.AnswerText);
                }
                else {
                    throw new Exception("Missing Answer for questionid" + questionId);
                }
                Random rnd = new Random();

                while (GetPossibleAnswerCombination.Count < 3) {

                    var randomIndex = rnd.Next(0, Anslibrary.Count - 1);
                    var GetRandomPossibleAnswer = Anslibrary[randomIndex];
                    if (GetRandomPossibleAnswer.Question.QuestionId != questionId && !GetPossibleAnswerCombination.ContainsKey(GetRandomPossibleAnswer.AnswerId)) {
                        GetPossibleAnswerCombination.Add(GetRandomPossibleAnswer.AnswerId, GetRandomPossibleAnswer.AnswerText);
                    }
                }
                var sheffuleAnswer = GetPossibleAnswerCombination.Values.OrderBy(x => Guid.NewGuid()).ToArray();
                ans = string.Join(" ,", sheffuleAnswer);
            }
            catch (Exception ex) {
                ex.ToString();
            }
        }
        return ans;
    }

    public double GetMarks(List<Questionnaire> qList) {

        var AnsLibrary = GetAnswerLibrary();

        double result = 0;
        double TotalNumberofQuestion = qList.Count();
        int UserValidAnswer = 0;

        try {
            if(TotalNumberofQuestion > 0) {

                var filterQuestionaire = qList.Where(q => q.UserAnsResponse != null).ToList();

                foreach(var q2 in filterQuestionaire) {


                    bool checkOnlyAnswerQuestion = AnsLibrary.Any(x => x.Question.QuestionId == q2.QuestionId);

                    if(checkOnlyAnswerQuestion) {

                        string city = (q2.UserAnsResponse).Trim();
                        bool checkifValidAnswer = AnsLibrary.Any(x => x.Question.QuestionId == q2.QuestionId && x.AnswerText == city);


                        if(q2.isCorrectAnswer == true && checkifValidAnswer == true) {
                            UserValidAnswer++;
                        }

                        if(q2.isCorrectAnswer == false && checkifValidAnswer == false) {
                            UserValidAnswer++;
                        }
                    }
                }
                result = UserValidAnswer / TotalNumberofQuestion * 100;
            }
        } catch(Exception ex) {

            ex.ToString();
        }
        return result;
    }
}

Answer 1

一些简短的评论：

• 问卷是一系列的问题，但你似乎把它误认为是一个问题。
• _questionId：参数不应该以下划线开头。
• Question真的应该有一个QuestionId吗？为什么不简单地使用Id呢？
• UserAnsResponse是个奇怪的名字。
• QuestionAsk是个奇怪的名字。
• isCorrectAnswer是一个属性，因此应该是PascalCase应该是PascalCase。
• qList是参数的坏名称。首先，您不必要地缩写了Questionnaire。此外，如果你把它变成一个IEnumerable<T>，你会重新命名这个参数吗，因为它不再是一个List<T>了？只需将其命名为questionnaires。
• 对我来说，GetAnswerOptions建议它返回一个集合，但它返回一个字符串。结果存储在属性PossibleAnswer中，该属性不建议使用集合。这一切都很令人困惑。
• AnsLibrary不必要地缩写，而且应该是camelCase。(但我想这些在“现实生活”中不会出现。)
• GetQuestionnaire建议它返回一个问卷，但它返回一个List<Questionnaire>。
• Anslibrary是不必要的缩写；而且它应该是camelCase，“库”应该大写，因为它是复合词。
• GetPossibleAnswerCombination是方法的名称，而不是变量的名称。
• CheckIsAnswerExisr是一个局部变量，因此应该是camelCase，加上它包含两个输入。
• GetRandomPossibleAnswer：变量的名字不是很好，加上它应该是camelCase。
• sheffuleAnswer：包含排版。

GetAnswerOptions的整个逻辑设计得很糟糕。

• 为什么要做CheckIsAnswerExisr然后做Where？
• 你想要做的事情--得到问题的一个有效答案--可以很容易地使用.SingleOrDefault()来完成。使用.FirstOrDefault()是严重错误的，因为对这个问题不应该有多个正确的答案。此外，您甚至不检查ValidAnswer是否为null，这可能是.FirstOrDefault()的结果。
• 你从Anslibrary那里得到一个随机的答案，然后才检查它是否属于这个问题？然后检查它是否已经在GetPossibleAnswerCombination中了？
• 如果一个问题没有答案，那么整个try...catch就可以捕获您抛出的异常，这太过分了。

但实际上:抛开它，完全重新思考逻辑以及你需要如何实现它。

转到GetMarks：

• 为什么会有try...catch？
• 不要不必要地缩写：qList。
• 为什么分配filterQuestionaire而只使用它一次？
• 同样，您首先执行一个Any()，然后执行一个Where()。只需做Where()，然后从那里出发。
• 你为什么要检查q2.isCorrectAnswer？不管怎么说，它从来没有设定过！

坦白地说，我只是在触及表面而已。IMHO --您的解决方案的基本设计是有缺陷的，所有这些都只是症状。

Answer 2

你必须有很大的垂直间距(新行)，这降低了可读性。

变量名不应缩短(List<Answer> AnsLibrary)。此外，没有关于变量的大小写样式的约定，这些变量是方法的本地变量，您应该使用camelCase大小写。

此外，你应该坚持你所选择的风格。现在，您正在混合局部变量的大小写。

此外，对于在哪里放置一个开口大括号{也没有约定，大多数C#开发人员都希望他们在新的线路上。

如果你保持你的风格，你至少应该坚持下去。现在你正在混合这两种风格。

应该通过使用如下的GetAnswerOptions()子句来改进

if (Anslibrary.Count == 0) { return string.empty; }  

这将减少水平间距，从而增加可读性。

Random应该创建一次，然后应该被重用。

捕获(异常ex) { ex.ToString()；}

这并不会增加代码的任何价值。如果您想要吞下一个异常，那么应该用一个注释来替换ex.ToString();，说明为什么要吞下该异常。

GetMarks()方法也应该通过添加保护子句来改进。

这

如果(q2.isCor矩形== true && checkifValidAnswer == true) { UserValidAnswer++；}if(q2.isCor矩形== == & checkifValidAnswer == false) { UserValidAnswer++；}

可以简化为

if(q2.isCorrectAnswer == checkifValidAnswer) 
 {
    UserValidAnswer++;
}