从根音符产生音乐音阶

Question

我想提高我的编码技能。我对编程很陌生，经过数周的研究、试验和错误--我已经成功地编写了这个基本程序，通过输入根音符和音阶来找到音乐音阶。

因为我几乎是在自学--我在寻找批评来改进我的语法/编程。我压缩了下面列出的代码和局部变量到这个程序的46行版本，但真的很喜欢原来的布局，所以请残忍地诚实。专业人士和同事请给我解释一下？

注意:我纯粹是懒散地使用#define，并计划用正确的std::typename替换所有字符串和向量。

#include <iostream>
#include <string>
#include <vector>
#include <map>

#define astring std::string
#define cvector std::vector

astring rootnote = "";
astring scale = "";
void notation();
int root;
cvector<astring> notes;
cvector<astring> order;
void scaler();


int main()
{
    while (rootnote != "null") {

        std::cout << "Please enter your root note and scale: " << std::endl;
        std::cin >> rootnote >> scale;

        std::cout << "\nroot scale: " << rootnote << " " << scale << std::endl;
        std::cout << std::endl;

        notation();
        scaler();
    }

    return 0;
}

void notation()
{
    notes.clear();

    cvector<astring> chromatic = { "C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab", "A", "Bb", "B" };

    root = std::distance(chromatic.begin(), std::find(chromatic.begin(), chromatic.end(), rootnote));

    for (int i = root; i < chromatic.size(); i++) { notes.push_back(chromatic[i]); }
    for (int j = 0; j < root; j++) { notes.push_back(chromatic[j]); }

    return;
}

void scaler() {
    order.clear();

    std::map<astring, cvector<int>> scales;
    scales["Major"] = { 0, 2, 4, 5, 7, 9, 11 };
    scales["Minor"] = { 0, 2, 3, 5, 7, 8, 10 };
    scales["Melodic"] = { 0, 2, 3, 5, 7, 9, 11 };
    scales["Harmonic"] = { 0, 2, 3, 5, 7, 8, 11 };

    for (auto i : scales[scale])
        order.push_back(notes[i]);
    for (int j = 0; j < order.size(); j++)
        std::cout << order[j] << " ";

    std::cout << "\n\n" << std::endl;

    return;
}

Answer 1

这些全局变量构成了意大利面的代码。函数应该明确它们的输入参数是什么，并返回它们的输出值。与其使用全局变量，不如将所有内容集中到一个main()函数中。具有讽刺意味的是，本应是全局常量的两个变量-- chromatic和scales --却并非如此。

通常将main()放在最后，而助手函数放在第一位，这样您就不必写前写声明了。

您使用了std::find()和std::distance()，所以您应该使用#include <algorithms>和<iterator>。我建议按字母顺序列出你的资料。

行为

Please enter your root note and scale: 
F# Major

root scale: F# Major

C D E F G A B 

如果在rootnote中找不到chromatic，则应该处理这种情况，而不是盲目地基于chromatic.end()进行处理。

Please enter your root note and scale: 
D Major

root scale: D Major

D E Gb G A B Db 

这些同义词听起来是一样的，但技术上应该是F♯和C♯。

Please enter your root note and scale: 
null blah

root scale: null blah

让用户输入神奇的单词"null"作为退出程序的根注释是很奇怪的。

建议的解决方案

#include <algorithm>
#include <iostream>
#include <iterator>
#include <map>
#include <sstream>
#include <string>
#include <vector>

const std::vector<std::string> chromatic = {
    "C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab", "A", "Bb", "B"
};

const std::map<const std::string, const std::vector<int>> degrees = {
    {"Major",       { 0, 2, 4, 5, 7, 9, 11, 12 }},
    {"Minor",       { 0, 2, 3, 5, 7, 8, 10, 12 }},
    {"Harmonic",    { 0, 2, 3, 5, 7, 8, 11, 12 }},
    {"Melodic",     { 0, 2, 3, 5, 7, 9, 11, 12 }}
};

std::string scalenotes(const std::string& rootnote, const std::string& scale) {
    int root = std::distance(chromatic.begin(), std::find(chromatic.begin(), chromatic.end(), rootnote));
    if (root >= chromatic.size()) {
        return "";
    }
    std::stringstream ss;
    for (int i : degrees.at(scale)) {
        ss << chromatic[(root + i) % chromatic.size()] << " ";
    }
    return ss.str();
}

std::stringstream ask(const std::string& question) {
    std::string line;
    std::cout << question;
    std::getline(std::cin, line);
    return std::stringstream(line);
}

int main()
{
    std::string rootnote, scale;
    while (ask("Please enter your root note and scale: ") >> rootnote >> scale) {
        std::cout << "\nroot scale: " << rootnote << " " << scale
                  << ": " << scalenotes(rootnote, scale) << "\n\n";
    }
}

Answer 2

1. 您应该避免全局向量变量，而是通过引用将其传递到函数中。我将删除所有全局变量，并将它们传递给函数。
2. 输入有点奇怪。您可以使用getline进行不同的操作。它不应该检查您是否输入“null”。如果你只传递一根线，它会爆炸，我很确定。如下所示: string行；while (getline(std::cin，line)) { // line包含输入的整个行用户}
3. 保持for循环格式和括号一致。
4. 没有理由为向量和字符串定义不同的名称，只是让其他开发人员感到困惑。如果您不想键入std::vector，std::cout每次使用它，您可以在顶部声明“使用std::cout”

除此之外，它似乎相当简单。干得好:D

反对的主要内容是全局变量，而不是通过函数传递引用，以及如何获得用户输入。