Linker error: multiple definitions

[amccart]

Dear Fellow Programmers,

I've tried to organize the files according to best practices:
declarations all in "pitch.h"
definitions in "pitch.cpp"
program begins and ends in "main.cpp"

I'm getting a bunch of linker errors when I compile just like this one:
Error LNK2005 "public: __cdecl FisMoll::Pitch::Pitch(enum FisMoll::ePitchFisMoll)" (??0Pitch@FisMoll@@QEAA@W4ePitchFisMoll@1@@Z) already defined in main.obj recursionOfTwoPathsInAPitchField C:\Users\Adam\source\repos\recursionOfTwoPathsInAPitchField\recursionOfTwoPathsInAPitchField\pitch.obj 1

I'll outline a description of the code as it appears in the three files below
and also give a short overview of the program and what it's doing. The definitions are a bit long winded, so I'll abbreviate where possible.

A simple program that models the path traveled by individual pitches of a melody as they are transposed through a pitch field. In other words, these pitches are transposed by a set of diatonic intervals a number of times and the number of times that a specific pitch occurs is counted. The idea is an attempt to model the types of recursive algorithms used to generate strange attractors and other similar types of chaotic symmetries.

As in it's a music by numbers generator. Recurring pitches will be colored according to a statistical distribution. Color in this case refers to the use of a specific type of orchestration to differentiate between pitches. That's not part of the program, but maybe it helps to know.

Code has been designed using a pseudocode programming process, so the comments *should* convey a detailed description of the idea and what's happening.

A fresh pair of eyes and any pointers much appreciated!

pitch.h

  #pragma once

#include "c:\Users\Adam\Documents\pppuc++\std_lib_facilities.h"

namespace FisMoll {

// Create pitch space
enum class ePitchFisMoll {
	fis = 1, gis, a, b, bis, cis, d, dis, e, f
};

class Pitch {
public:	
	Pitch();                // default constructor 
	Pitch(ePitchFisMoll p); // initialize with pitch from F# minor and 1 steps

	// nonmodifying operations
	string GetPitchAsString(Pitch p);
	ePitchFisMoll GetCurrentPitch();
	string GetCurrentPitchString();

	// modifying operations
	ePitchFisMoll TransposeUpFifth(Pitch p);
	//...

private:
	int steps = 1;
	ePitchFisMoll current_pitch = {ePitchFisMoll::fis};
	string current_pitch_string = {"fis"};

};


}// FisMoll 

pitch.cpp

#include "pitch.h"
#include <iterator>

namespace FisMoll {

	// Create memory bank of pitches past
	unordered_map<string, int> FisMemPath({
		{"fis",0},{"gis",0},{"a",0},{"b",0},{"bis",0},{"cis",0},{"d",0},{"dis",0},{"e",0},{"f",0}
		});

	// increment map value for key 'c'
	void SetStepCountFisPathMem(string c) {
		FisMemPath[c]++;
	};

	//-------------------------------------------------------------
	// construct PitchClass

	Pitch::Pitch(ePitchFisMoll pp) : current_pitch{ pp } { } // initializes pitch with a member of enum class Fis Moll

	//--------------------------------------------------------------

	// Function definitions

	ePitchFisMoll Pitch::GetCurrentPitch() { return current_pitch; }

	string Pitch::GetCurrentPitchString(){ return current_pitch_string; }

	void Pitch::SetCurrentPitch(Pitch cp) { current_pitch = cp.GetCurrentPitch(); }
	void Pitch::SetCurrentPitchString(Pitch cp) { current_pitch_string = cp.GetCurrentPitchString(); }

	//-------------------------------------------------------------------------------------------

	string Pitch::GetPitchAsString(Pitch p) {
		switch (p.GetCurrentPitch()) { // returns the current pitch from ePitchFisMoll
		case ePitchFisMoll::fis:
			return "fis";              // return transposed pitch
			break;
		case ePitchFisMoll::gis:
			return "gis";
			break;
		case ePitchFisMoll::a:
			return "a";
			break;
		case ePitchFisMoll::b:
			return "b";
			break;
		case ePitchFisMoll::bis:
			return "bis";
			break;
		case ePitchFisMoll::cis:
			return "cis";
			break;
		case ePitchFisMoll::d:
			return "d";
			break;
		case ePitchFisMoll::dis:
			return "dis";
			break;
		case ePitchFisMoll::e:
			return "e";
			break;
		case ePitchFisMoll::f:
			return "f";
			break;
		}
	};


	//--------------------------------------------------------------	

	// Define transposition functions
	//
	// up a fifth()
	ePitchFisMoll Pitch::TransposeUpFifth(Pitch p) {
		// switch over input
		switch (p.GetCurrentPitch()) {
		//...
		}

	};



	// up a third()
	ePitchFisMoll Pitch::TransposeUpThird(Pitch p) {
		// switch over input
		switch (p.GetCurrentPitch()) {
		//...
	};

	// down a sixth()
	ePitchFisMoll Pitch::TransposeDownSixth(Pitch p) {
		// switch over input
		switch (p.GetCurrentPitch()) {
		//...
	};
	//------------------------------------------------------------------

	// Define Path One (ePitchFisMoll p)
// Move the pitch along path
	void Pitch::SetPositionOnPathOne(Pitch p) {  
// count steps in local path (path is a short, recursive loop)
		int step_counter = 0;                    
		ePitchFisMoll trans_pitch;
		while (step_counter < 4) {
			if (step_counter < 3) {                                    
				trans_pitch = TransposeUpFifth(p.GetCurrentPitch());   
				step_counter++;
				p.IncrementSteps();                                    
				p.SetCurrentPitch(trans_pitch);                        
				SetCurrentPitchString(p);                              
				SetStepCountFisPathMem(p.GetCurrentPitchString());     
			}
			else if (step_counter < 4) {                               
				trans_pitch = TransposeUpThird(p);                     
				step_counter++;                                        
				p.IncrementSteps();
				p.SetCurrentPitch(trans_pitch);
				SetCurrentPitchString(p);                               

// add pass of new pitch to memory of pitches past			
SetStepCountFisPathMem(p.GetCurrentPitchString());     
			}
			else break;
		}
	};

	//--------------------------------------------------------------


	// Steph through each path recursively

	// for x iterations
	// track pitch p along pathOne
        // initialize this function with a starting pitch
	void Pitch::TracePathOne(Pitch init_pitch) {                   
		for (int i = 0; i < 5000; ++i) {                           
			init_pitch.SetCurrentPitch(init_pitch);                
			init_pitch.SetPositionOnPathOne(init_pitch);       
		}
	};

	// follow pathTwo(pitch p)

	//------------------------------------------------------------------
	

}// Fis Moll

main.cpp

#include "pitch.cpp"

int main()
try {
	using namespace FisMoll;

	Pitch PPathOne(ePitchFisMoll::fis);  // instantiate initial pitch on PathOne
	PPathOne.TracePathOne(PPathOne);
	for (auto& e : FisMemPath) {
		cout << '{' << e.first << ", " << e.second << '}' << '\n';
	};
	

}
catch (exception& e) {
	cerr << "exception: " << e.what() << endl;
}
catch (...) {
	cerr << "exception\n";
}

[dutch]

Your code is riddled with errors. E.g., SetCurrentPitch (amongst others) is not declared in the class definition.

Compile the program.
Then scroll back to the very first error/warning listed and fix that.
Repeat.

You shouldn't be using std_lib_facilities.h. It just complicates things. And it seems to have errors in it, too.

[amccart]

@dutch thanks for your response...

The code was too long to fit in the form, so abbreviated the class
definition and in many other places all indicated with "//..."
you can see the complete code here:
https://github.com/adammccartney/RecursionOfTwoPathsInAPitchField

Will get rid of the std_lib_facilities.h, and build the thing from scratch
again, copying in the definitions and functions one by one to catch
the errors as they come up.

[amccart]

okay, so narrowed the error down to the following:

Error LNK2005 "public: __thiscall FisMoll::Pitch::Pitch(enum FisMoll::ePitchFisMoll)" (??0Pitch@FisMoll@@QAE@W4ePitchFisMoll@1@@Z) already defined in main.obj RecursivePitch C:\Users\Adam\source\repos\RecursivePitch\RecursivePitch\pitch.obj 1

pitch.h

#pragma once

#include <iostream>
#include <string>


namespace FisMoll {

	// Create pitch space
	enum class ePitchFisMoll {
		fis = 1, gis, a, b, bis, cis, d, dis, e, f
	};

	class Pitch {
	public:
		Pitch();                // default constructor 
		Pitch(ePitchFisMoll p); // initialize with pitch from F# minor and 1 steps

		// nonmodifying operations
		ePitchFisMoll GetCurrentPitch();
		std::string GetCurrentPitchString();

	private:
		int steps = 1;
		ePitchFisMoll current_pitch{ ePitchFisMoll::fis };
		std::string current_pitch_string{ "fis" };

	};

	std::ostream& operator<<(std::ostream& os, const Pitch& p);

};//FisMoll

pitch.cpp

#include "pitch.h"

namespace FisMoll {
	//-------------------------------------------------------------
	// construct PitchClass

	Pitch::Pitch(ePitchFisMoll pp) : current_pitch{ pp } { } // initializes pitch with a member of enum class Fis Moll

	//--------------------------------------------------------------

	// Function definitions
	ePitchFisMoll Pitch::GetCurrentPitch() { return current_pitch; }
	std::string Pitch::GetCurrentPitchString() { return current_pitch_string; }


};

main.cpp

#include "pitch.cpp"
#include <stdexcept>   // for exception, runtime_error, out_of_range

#include <iterator>

int main()
try {
	using namespace FisMoll;

	Pitch TestPitch{ ePitchFisMoll::fis };
	std::cout << "here is the current pitch: " << TestPitch.GetCurrentPitch() << '\n';

}
catch (std::runtime_error& e) {
	std::cout << "Caught a runtime_error exception: "
		<< e.what() << '\n';
}
catch (std::exception& e) {
	std::cout << "Caught an exception of an unexpected type: "
		<< e.what() << '\n';
}
catch (...) {
	std::cout << "Caught an unknown exception\n";
}

[dutch]

In main.cpp you should include pitch.h, not pitch.cpp.
And you need to define your operator<< overload in pitch.cpp.

[amccart]

Super, thanks! I've actually stripped it back even further and gotten back to a single error ... same as above.

So the two objects being created are pitch.obj and main.obj,
is this correct?

I've created a wrapper for the code in pitch.h, as I need to include it twice,
once in pitch.cpp and once in main.cpp

Am I doing something idiotic while trying to construct a Pitch object?

pitch.h

#pragma once
#ifndef PITCH_GUARD
#define PITCH_GUARD

#include <iostream>
#include <string>


namespace FisMoll {

	// Create pitch space
	enum class ePitchFisMoll {
		fis = 1, gis, a, b, bis, cis, d, dis, e, f
	};

	class Pitch {
	public:
		Pitch();                // default constructor 
		Pitch(ePitchFisMoll p); // initialize with pitch from F# minor and 1 steps

	private:
		int steps{ 1 };
		ePitchFisMoll p;
		std::string current_pitch_string;
		bool is_valid();

	};

	//std::ostream& operator<<(std::ostream& os, const Pitch& p);

};//FisMoll
#endif 

pitch.cpp

#include "pitch.h"

namespace FisMoll {
//-------------------------------------------------------------
// construct PitchClass

Pitch::Pitch(ePitchFisMoll p) : p{ p } { } // initializes pitch with a member of enum class Fis Moll


//---------------------------------------------------------------------------------------

};//FisMoll



main.cpp

#include "pitch.h"
#include <stdexcept>   // for exception, runtime_error, out_of_range

int main()
try {
	using namespace FisMoll;

	Pitch TestPitch;
	//std::cout << "here is the current pitch: " << TestPitch.CurrentPitch() << '\n';

}
catch (std::runtime_error& e) {
	std::cout << "Caught a runtime_error exception: "
		<< e.what() << '\n';
}
catch (std::exception& e) {
	std::cout << "Caught an exception of an unexpected type: "
		<< e.what() << '\n';
}
catch (...) {
	std::cout << "Caught an unknown exception\n";
}

[dutch]

Your current code is attempting to use the default ctor (the one with no arguments), but you haven't defined that one.

[amccart]

Okay, again super helpful, thank you!

I got a test version working with both versions of the constructor where the pitch is an int instead of the enum class.

My incling is that another problem arises when I get the enum class defined and in.

At the moment I'm having trouble defining the << operator

std::ostream& operator<<(std::ostream& os, Pitch& p) 
	{
		return os << p.CurrentPitch() << '\n';
	}

[dutch]

If you want to print it as an int then cast it to an int.

return os << static_cast<int>(p.CurrentPitch() );

If you want to print it as a string, then maybe something like:

const char* pitch_names[] = { "unknown", "fis", "gis", "a", "b", "bis", "cis", "d", "dis", "e", "f" };
return os << pitch_names[static_cast<int>(p.CurrentPitch())];

[amccart]

@dutch

excellent stuff, thanks again for your help, I'd like to avoid static_cast definitions,
but I understand the point that you are trying to make. I have a version printing
to screen now with a member function that returns a string.

That seems to be the initial mess untangled to some degree.
I'm going to step through the rest of the functions and see if I can get the program up and running.

Will post again here when I manage to get it going.

Thanks for all your help and happy new year!

[amccart]

okay, problem solved...the error I was getting was due to the multiple definition of an <unordered_map> object.
There were two ways of fixing this -
either by prefixing the declaration within pitch.h with extern to make the variable global
or
(better practice) by including the variable as a class member withing PitchClass.

Also, found that I was hardcoding in a bunch of unneccessary stuff, so redesigned the whole lot while error checking the functions.

pitch.h

#ifndef PITCH_GUARD
#define PITCH_GUARD

#include <iostream>
#include <string>
#include <unordered_map>


namespace PitchPath {

	
	// Create pitch space
	enum class Pitch { c, cis, d, dis, e, f, fis, g, gis, a, b, h };
	
	class PitchClass {
	public:
		class Invalid {};

		PitchClass();                               // default constructor 
		PitchClass(int p, std::vector<int> tp);     // initialize with starting pitch and transposition steps 

		// nonmodifying operations
		int GetPitch() { return p; };
		void PrintRecursionMap();

		//modifying operations
		void SetPitch(int pp) { p = pp; }
		void SetTransposition(std::vector<int> trans) { tp = trans; }
		void CountPitchRecursion(int p) { recursion_map[p]++; }
		
		// transpose for every member of transposition vector, move 
		void Transpose(const int& p, const std::vector<int>& tp, int x);
		void AddPitch(int n);

	private:
		int p;
		std::vector<int> tp;
		std::string current_pitch_string;
		std::unordered_map<int, int> recursion_map;
		//bool is_valid();
	
	};


};//PitchPath
#endif 

pitch.cpp

#include "pitch.h"

namespace PitchPath {

	Pitch operator++(Pitch& p)
	{
		p = (p == Pitch::h) ? Pitch::c : Pitch(int(p) + 1); // wrap around
		return p;
	}
	
	//-------------------------------------------------------------
	// construct PitchClass

	// default
	PitchClass::PitchClass() : p{ 0 }, tp{ {'\0'} } { }
	PitchClass::PitchClass(int p, std::vector<int> tp) 
		: p{ p }, tp{ tp }
	{
		//if (!is_valid())throw Invalid{}; // if valid, initializes pitch with a member of enum class Fis Moll
	}

	void PitchClass::PrintRecursionMap()
	{
		for (auto& e : recursion_map) {
			std::cout << '{' << e.first << "," << e.second << '}' << '\n';
		}
	}


	//----------------------------------------------------------------

	/*
	bool PitchClass::is_valid()
	{
		if(p < 0 || 11 > p)return false; // there are 12 pitches in an octave
		{
			// if pitch is valid, initialize
			PitchClass::p = p;
			PitchClass::tp = tp;
		}
		return true;
	}
	*/
	//------------------------------------------------------------------------

	void PitchClass::AddPitch(int n)
	{
		bool lastPitch = false;
		// if day exceeds last pitch in the octave, roll on to next octave and set pitch to 0
		for (int i = 0; i < n; ++i)
		{
			if (p == 11)
				lastPitch = true;
				p = (p == 11) ? 0 : ++p; // at h, roll on to c if pitch = 11, otherwise ++
		}
	}

	//------------------------------------------------------------------------

	// arguments are starting pitch, tranpositions, number of interations
	void PitchClass::Transpose(const int& p, const std::vector<int>& tp, int x)  
	{
		for (int i = 0; i < x; ++i) {
			for (int j = 0; j < tp.size(); ++j) {
				AddPitch(tp[j]);
				CountPitchRecursion(GetPitch());
			}
		}
	}
};//PitchPath 

main.cpp

#include "pitch.h"
#include <stdexcept>   // for exception, runtime_error, out_of_range

int main()
try {
	using namespace PitchPath;

	int fis = 7;
	std::vector<int> fft= { 7,7,4 };
	PitchClass fisOne = { fis,fft };

	int e = 6;
	std::vector<int> ttff = { 4,4,7,7 };
	PitchClass eOne = { e,ttff };

	fisOne.Transpose(fis, fft, 100000);
	fisOne.PrintRecursionMap();

	eOne.Transpose(e, ttff, 100000);
	eOne.PrintRecursionMap();

}
catch (std::runtime_error& e) {
	std::cout << "Caught a runtime_error exception: "
		<< e.what() << '\n';
}
catch (std::exception& e) {
	std::cout << "Caught an exception of an unexpected type: "
		<< e.what() << '\n';
}
catch (...) {
	std::cout << "Caught an unknown exception\n";
}

Using c++ to do a job like this is overkill as a couple of colleagues have already pointed out. New years resolution = learn python ;)

Thanks for the help!