Problem with nested for loop

[BroBuilder]

I am trying to create an array of twenty-six objects of the class Animal. I want to have a function that names each object a letter of the alphabet starting with a and ending with z. I want to have a function that prints the names of each object, each on its own line. I was trying to loop through a for loop to establish the letter of the alphabet and then run through a nested for loop to name the object that letter. However, this is not working. As far as I can tell the first for loop never runs and neither does the nested one. The first for loop is supposed to turn charAlphabet into b before it runs a second time and the nested for loop is supposed to iterate through i so as to set the twenty-six objects each to a different letter of the alphabet. The second for loop states the name of each object, but they all come out as a.

#include <iostream>
using namespace std;

class Animal{
private:
	char name;
public:
	Animal(){cout << "constructor called." << endl; name = 'a';}

	Animal(const Animal& otherAnimal): name(otherAnimal.name) {cout << "animal created via copying.";};

	~Animal(){cout << "destructor called." << endl;}

	void setName(char name){this->name = name;};
	void speakName(){cout << name << endl;};

	Animal* allocateMemoryForAnimalArray(){
		Animal* animal = new Animal[26];

		return animal;
	};

	void deallocateMemoryForAnimalArray(char* memoryToBeDeallocated){
		delete [] memoryToBeDeallocated;};
};

int main() {

	Animal* animal = new Animal[26];

	for(char charAlphabet='a';charAlphabet=='z';charAlphabet++){
		cout << charAlphabet << flush;
		for(int i=0;i<26;i++){
			animal[i].setName(charAlphabet);
		}
	}

	for(int i=0;i<26;i++){
		animal[i].speakName();
	}

	delete animal;
	
	return 0;
}

[Handy Andy]

Hello BroBuilder,

Thank you for using code tags.

In "mamin" your for loop says: charAlphabet=='z' since you start at "a" this is not equal to 'z' and therefor it is false and the for loop never runs. Consider this:

int idx{};

for (char letter = 'a'; letter <= 'z'; letter++)
{
    cout << letter << '\n';

    animals[idx++].setName(letter);
}

I do not like the "<=", but sometimes it is necessary.

The inner for loop does not work and is not needed.

The "delete" at the end of "main" is 1/2 right. you need to delete an array not just a single. delete[] animal;

These 2 functions in the class are not needed.

allocateMemoryForAnimalArray()
deallocateMemoryForAnimalArray(char* memoryToBeDeallocated)

But you did get the delete correct in the 2nd function.

A suggestion: Since you are creating an array call the variable "animals". This sounds better as an array name than just a single "animal".

If you do that the 2nd for loop will have something to display on the screen.

Andy

[JLBorges]

> for (char letter = 'a'; letter <= 'z'; letter++)

This is not a good idea; it will work as expected on many implementations, but fail on others, for example one which uses an EBCIDIC encoding.

The gaps between letters made simple code that worked in ASCII fail on EBCDIC.
For example for (c = 'A'; c <= 'Z'; ++c) putchar(c); would print the alphabet from A to Z if ASCII is used, but print 41 characters (including a number of unassigned ones) in EBCDIC
https://en.wikipedia.org/wiki/EBCDIC#Compatibility_with_ASCII

Consider using std::vector or std::array; avoid calling new and delete explicitly: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-newdelete

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

class Animal {

    private: char name_;

    public:

        Animal( char name = 0 ) noexcept : name_(name) {}

        void name( char new_name ) noexcept { name_ = new_name; }
        char name() const noexcept { return name_ ; }

        void speak() const { std::cout << *this << '\n' ; }

	friend std::ostream& operator<< ( std::ostream& stm, Animal a ) {

	    return stm << "animal{"<< a.name_ << '}' ;
	}
};

int main() {

    // vector: https://cal-linux.com/tutorials/vectors.html
    // Prefer using STL array or vector instead of a C array:
    //     https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rsl-arrays
    std::vector<Animal> animals ; // create an empty vector

    // string to hold animal names
    // note: The values of the members of the execution character sets
    //       are locale-specific. https://eel.is/c++draft/lex.charset#3
    //       so we can't portably assume that character b == a+1, c == b+1 etc.
    const std::string names = "abcdefghijklmnopqrstuvwxyz" ;

    // add the animals one by one to the vector
    // range based loop: https://www.stroustrup.com/C++11FAQ.html#for
    for( char c : names ) animals.emplace_back(c) ; // https://en.cppreference.com/w/cpp/container/vector/emplace_back

    // and print them out
    for( const Animal& a : animals ) std::cout << a << '\n' ;
}

http://coliru.stacked-crooked.com/a/879cc5232e79291d

[seeplus]

Is EBCDIC encoding still used? I thought it was something IBM had for their mainfames to make them different and incompatible with their competitors...

As the size of the vector is known, it's capacity can be reserved.

[JLBorges]

EBCDIC is still used by IBM mainframes and mid-range machines (and compatibles like Fujitzu).

The important point is that the C++ standard does not guarantee that the loop
for (char letter = 'a'; letter <= 'z'; letter++)
would work as the programmer expects it to work.

[helios]

This is not a good idea; it will work as expected on many implementations, but fail on others, for example one which uses an EBCIDIC encoding.

What's not a good idea is wasting developer time adding complexity to handle situations that will never come up.

[againtry]

#include <iostream>

using namespace std;

class Animal
{
private:
    char m_name;
public:
    Animal(){}
    ~Animal(){}

    void setName(char name){m_name = name;}
    void speakName(){cout << m_name << ' ';}
};

int main()
{
    Animal* animal = new Animal[26];

    for(int i = 0; i < 26; i++)
    {
        animal[i].setName('a' + i);
    }

    for(int i=0; i < 26; i++)
    {
        animal[i].speakName();
    }
    cout << '\n';

    delete[] animal;
    return 0;
}

a b c d e f g h i j k l m n o p q r s t u v w x y z 
Program ended with exit code: 0

[BroBuilder]

Thank you all for your prompt replies. As I do not see an individual reply option, I shall reply to each of you in this message.

To Handy Andy: Your reply was quite helpful. I did, however, make a couple of small changes to your code. The functional thing that I changed was I got rid of the cout. the goal of my project was to print the names of the objects. Your code produced the same effect in the console, but it wasn't the way in which I wanted the result. Also, thank you for the suggestion about the variable name. I took it. The problem I had with the for loop was that I thought that charAlphabet == 'z' would make the loop run until charAlphabet was equal to 'z'.

To JLBorges: Thank you for your reply. You obviously took a fair bit of time on this response and for that I thank you. I am a beginner and have some questions regarding some of the things you said/used. I read the section of the article on not explicitly calling new and delete: what is this leak the article talks of? Would my to functions:

Animal* allocateMemoryForAnimalArray(){
		Animal* animal = new Animal[26];

		return animal;
	};

	void deallocateMemoryForAnimalArray(char* memoryToBeDeallocated){
		delete [] memoryToBeDeallocated;};

fix this problem? Also, at one point, before I posted to this forum, I tried replacing lines 29 and 42 with these functions, but it didn't work. The main function wouldn't recognize animal when I did this. Why is that?
Thank you for the resources.

To helios: Thank you for your reply. I see your point. I am a beginner and am just doing this for a challenge. However, I do want to improve at coding and learn more. That being said, JLBorges' reply is still useful to me, espicially since he included resources to help me figure out what he did.

To againtry: Thank your for your reply also. I like your for loop that uses i to run the loop twenty-six times and name the objects. Your code is concise. When I put your code in my compiler it gave me a message saying that m_name was not initialized in the constructor. It does not count this message as a warning and the code works. Why is it displaying this message?

Thank you all for your assistance, BroBuilder.

[againtry]

Why is it displaying this message?

It is good practice, but not mandatory to set an initial (default) value for all variables. In this case ‘?’ might be a good choice.
This message is a warning only and the program compiled and ran successfully, if it showed as an error compiling would continue as best as possible but would not run.

[TheIdeasMan]

@BroBuilder

https://en.wikipedia.org/wiki/Memory_leak

Also read the section on RAII.

One of the main problems with using new is that if an exception is thrown, the delete is never reached.

https://en.wikipedia.org/wiki/Exception_handling
https://en.wikipedia.org/wiki/Exception_handling#Exception_support_in_programming_languages

So avoid calling new and delete.

Often beginners use new just to obtain a pointer. This is one of my pet hates :+)

Also new is used to allocate memory on the heap. But it is better to use an STL container instead, which implicitly puts it's data on the heap. If one can't do that, then use a smart pointer such as std::unique_ptr or std::shared_ptr

[JLBorges]

> what is this leak the article talks of? Would my to functions ... fix this problem?

Wiki:

a memory leak is a type of resource leak that occurs when a computer program incorrectly manages memory allocations in a way that memory which is no longer needed is not released. A memory leak may also happen when an object is stored in memory but cannot be accessed by the running code.
https://en.wikipedia.org/wiki/Memory_leak

If every part of the code calls your functions correctly (that is, every new is correctly matched with a corresponding delete), there would be no leak. It is hard to do this consistently in non-trivial programs; ergo in C++ we use RAII: https://en.wikipedia.org/wiki/Memory_leak#RAII

The suggestion to use std::vector<> (use RAII) was not because it is hard to manage resources correctly in a very small program consisting of a few lines of code. As we start writing larger programs, complexity increases and it becomes harder to do this consistently.

Sutter and Alexandresu in 'C++ Coding Standards: 101 Rules, Guidelines, and Best Practices'

Use vector and string instead of arrays.

Summary
Why juggle Ming vases? Avoid implementing array abstractions with C-style arrays, pointer arithmetic, and memory management primitives. Using vector or string not only makes your life easier, but also helps you write safer and more scalable software.

Discussion
Buffer overruns and security flaws are, hands down, a front-running scourge of today's software. Silly limitations due to fixed-length arrays are a major annoyance even when within the limits of correctness. Most of these are caused by using bare C-level facilities—such as built-in arrays, pointers and pointer arithmetic, and manual memory management—as a substitute for higher-level concepts such as buffers, vectors, or strings.

> Also, at one point, before I posted to this forum,
> I tried replacing lines 29 and 42 with these functions, but it didn't work

Without seeing the code that failed, I'm guessing:
The functions as written are non-static member functions of Animal.
To be able to call them without an object of type Animal, we would need to make them static member functions.
https://www.tutorialspoint.com/cplusplus/cpp_static_members.htm

[BroBuilder]

Thank you all.