Return Pointer of Last Char

[dakotad8218]

Trying to figure out a helper function that'll return a pointer to the last character in a given string (in this case declared as a char[]).

Does this look correct? I'm not sure how to check the output on this one.

#include <iostream>
#include <cstring>
#include <cstdlib>


using namespace std;

char* find_last(char s[], char c)
{
    char* last = nullptr;
    char* p = s;
    for(int i = strlen(s); i > 0; i--)
    {
        if(*(p + i) == c)
        {
            last = p + i;
        }
    }

    return last;

}

int main()
{
    char str[] = "Hello";

    char* last = find_last(str, 'o');
    cout << last;
    return 0;
}

This just prints 'o' instead of a random memory address, which is what I was expecting.

[andywestken]

Try

cout << (void*)last;

to stop operator<< assuming you want it to display the null terminated string pointed to by last.

Andy

PS The address shouldn't be random, should it?

[andywestken]

And try e.g.

char str[] = "Hello, cplusplus.com";

comparing what you get with the value obtained using strrchr()

Andy

[dakotad8218]

I just need the function to return a pointer to the last occurrence of the character in the string... I guess 'l' would have been a better test of that. I only printed something to try to test and see if I could get it to spit out a memory address to verify that it's working correctly.

I mean, the address should be random between runs, at least that's what I was taught.

I thought by assigning a new pointer (*char last) that I'd be passing an address to cout << when I called cout << last. Wouldn't I call cout << *last if I wanted to print the actual char value? Pointers hurt my braaaaaaaaain

[TheToaster]

Operator<< for ostream has a non-member overload that accepts const char* and dereference the pointer for you. This is so std::cout can work with null terminated string literals. This is only the case for character pointers
https://en.cppreference.com/w/cpp/io/basic_ostream/operator_ltlt2

So your understanding is not wrong. It is correct. Technically, std::cout would return the address stored in last, if there hadn't been a special exception for const char* in the global ostream operator<<

[dakotad8218]

@TheToaster: Thanks for the clarification. So essentially it's just being converted to its dereferenced value when it's output with cout <<? last still contains the memory address? Would this be any different if I had used stdio.h instead?

[markyrocks]

i like this better.

#include <iostream>
#include <string>


using namespace std;

char* find_last(char s[], char c)
{
	string str = s;
	auto i = str.rfind(c);
	return &s[i];

}

int main()
{
	char str[] = "Hello";
	cout << (void*)find_last(str, 'o');
}

[andywestken]

@markyrocks

While it would be better to use std::string's rfind() method -- as it is a lot better to use std::strings than C-strings in most cases -- it doesn't make sense to use it to reimplement a standard C function (strrchr)

And here I am assuming dakotad8218 is doing an exercise, to implement the function ground-up. In that case your approach is cheating, as you're getting std::string to do all your hard work.

And it would be even more of a cheat to use:

char* find_last(char s[], char c)
{
	return strrchr(s, c);
}

Andy

[andywestken]

@dakotad8218

You have two issues in your code, as it stands at 11:15 on 24 May 2020...

Both of them require just minor tweaks to fix your code.

Andy

Output running test code on C++ Shell against find_last()

last 'o' in "Hello" => 0x400e28 ("o")
last 'l' in "Hello" => 0x400e26 ("llo")
last 'e' in "Hello" => 0x400e25 ("ello")
last 'H' in "Hello" => 0 (<null>)
last 'x' in "Hello" => 0 (<null>)
last 'o' in "Hello, cplusplus.com" => 0x400e2e ("o, cplusplus.com")
last 'l' in "Hello, cplusplus.com" => 0x400e2c ("llo, cplusplus.com")
last 'e' in "Hello, cplusplus.com" => 0x400e2b ("ello, cplusplus.com")
last 'H' in "Hello, cplusplus.com" => 0 (<null>)
last 'x' in "Hello, cplusplus.com" => 0 (<null>)

Whereas, when run against strrchr()

last 'o' in "Hello" => 0x400d68 ("o")
last 'l' in "Hello" => 0x400d67 ("lo")
last 'e' in "Hello" => 0x400d65 ("ello")
last 'H' in "Hello" => 0x400d64 ("Hello")
last 'x' in "Hello" => 0 (<null>)
last 'o' in "Hello, cplusplus.com" => 0x400d7c ("om")
last 'l' in "Hello, cplusplus.com" => 0x400d77 ("lus.com")
last 'e' in "Hello, cplusplus.com" => 0x400d6b ("ello, cplusplus.com")
last 'H' in "Hello, cplusplus.com" => 0x400d6a ("Hello, cplusplus.com")
last 'x' in "Hello, cplusplus.com" => 0 (<null>)

With added test code:

#include <iostream>
#include <cstring>
#include <cstdlib>

using namespace std;

// uncomment to run tests against strrchr rather than find_last
//#define CHECK_WITH_STRRCHR

void test_find_last();

char* find_last(char s[], char c);

int main()
{
    test_find_last();
    return 0;
}

char* find_last(char s[], char c)
{
    char* last = nullptr;
    char* p = s;
    for(int i = strlen(s); i > 0; i--)
    {
        if(*(p + i) == c)
        {
            last = p + i;
        }
    }

    return last;
}

// Test Code -- this works on C++ Shell but not Visual C++ 2019 :-\

void test_find_last()
{
    struct test_data
    {
        char* s;
        char  c;
    };

    static test_data data[] = {
        {"Hello", 'o'},
        {"Hello", 'l'},
        {"Hello", 'e'},
        {"Hello", 'H'},
        {"Hello", 'x'},
        {"Hello, cplusplus.com", 'o'},
        {"Hello, cplusplus.com", 'l'},
        {"Hello, cplusplus.com", 'e'},
        {"Hello, cplusplus.com", 'H'},
        {"Hello, cplusplus.com", 'x'},
    };

    static size_t test_data_count = sizeof(data)/sizeof(data[0]);

    for(size_t i = 0; i < test_data_count; ++i)
    {
        test_data& t = data[i];
#ifndef CHECK_WITH_STRRCHR        
        char* p = find_last(t.s, t.c);
#else
        char* p = strrchr(t.s, t.c);
#endif
        cout << "last \'" << t.c << "\' in \"" << t.s << "\" => " << (void*)p << " (";
        if(p == nullptr)
        {
            cout << "<null>";
        }
        else
        {
            cout << "\"" << p << "\"";
        }
        cout << ")" << endl;
    }
}

[andywestken]

// Test code adjusted to build with Visual C++ 2019
//
// 1. char* p = "Some const string" is now an error
//   so copy test strings into temp buffer to avoid changes to find_last()
// 2 strcpy() is deprecated, so #define _CRT_SECURE_NO_WARNINGS
//   to avoid need for secure version of function (so still runs on C++ Shell)

#define _CRT_SECURE_NO_WARNINGS
#include <iostream>
#include <cstring>
#include <cstdlib>

using namespace std;

// uncomment to run tests against strrchr rather than find_last
//#define CHECK_WITH_STRRCHR

void test_find_last();

char* find_last(char s[], char c);

int main()
{
    test_find_last();
    return 0;
}

char* find_last(char s[], char c)
{
    char* last = nullptr;
    char* p = s;
    for (int i = strlen(s); i > 0; i--)
    {
        if (*(p + i) == c)
        {
            last = p + i;
        }
    }

    return last;
}

// Test Code

void test_find_last()
{
    struct test_data
    {
        const char* s;
        char        c;
    };

    static const test_data data[] = {
        {"Hello", 'o'},
        {"Hello", 'l'},
        {"Hello", 'e'},
        {"Hello", 'H'},
        {"Hello", 'x'},
        {"Hello, cplusplus.com", 'o'},
        {"Hello, cplusplus.com", 'l'},
        {"Hello, cplusplus.com", 'e'},
        {"Hello, cplusplus.com", 'H'},
        {"Hello, cplusplus.com", 'x'},
    };

    static const size_t test_data_count = sizeof(data) / sizeof(data[0]);

    for (size_t i = 0; i < test_data_count; ++i)
    {
        const test_data& t = data[i];
        char s_buff[64]{};
        strcpy(s_buff, t.s);
#ifndef CHECK_WITH_STRRCHR        
        char* p = find_last(s_buff, t.c);
#else
        char* p = strrchr(s_buff, t.c);
#endif
        cout << "last \'" << t.c << "\' in \"" << t.s << "\" => " << (void*)p << " (";
        if (p == nullptr)
        {
            cout << "<null>";
        }
        else
        {
            cout << "\"" << p << "\"";
        }
        cout << ")" << endl;
    }
}

[keskiverto]

char* find_last(char s[], char c)
{
    char* last = nullptr;
    char* p = s;
    for(int i = strlen(s); i > 0; i--)
    {
        if(*(p + i) == c)
        {
            last = p + i;
        }
    }
    return last;
}

"find" usually returns the first match, i.e. breaks loop on hit.
"max_element" iterates the whole range to be sure that it finds the largest element.

You iterate the whole range. Every match updates the result. You thus return the last match.
You iterate in reverse order; from end to begin. Your "last match" is thus the first occurrence, not the last.

You could iterate in normal order iteration. Then last match is the last occurrence.
You could stop on first match in reverse iteration. That too gives the last occurrence.

The strlen() iterates the whole range in order to find the end. You call strlen() first and then loop. I.e. loop the string twice.

[TheToaster]

OP, yes last would still contain a memory address.

Would this be any different if I had used stdio.h instead

Potentially. If you use the "%p" format flag with printf, it will treat it as if it were just a pointer and give you the value (address). printf does not automatically deduce that it is a string constant for you. You have to specify explicitly using %s. Otherwise, if you use %p, it will just assume it is a regular pointer and give you its value. C++'s ostreams will abstract this for you and automatically deduce that if you pass a const char* you are expecting a string literal, which may not always be what you wanted. Example with printf:

#include <cstdio>

using namespace std;

int main()
{
    const char* string = "Hello World!";
    printf("%p",string); //Prints out ADDRESS

    return 0;
}

[markyrocks]

2
3
4
char* find_last(char s[], char c) { 	return strrchr(s, c); }

That seems like the best. I was not aware of that function. See you learn something every day.

I just based my suggestion off of what he said.

[keskiverto]

That seems like the best.

Definitely not.

* If the goal is to learn to use C, then one should not write any wrapper function and just use the strrchr() directly.
* If the goal is to learn to use C++, then one should use what std::string offers rather than having any C-strings.
* If the goal is to learn logical thinking, then one does not use existing library, just basic language constructs.

[markyrocks]

Yaya it doesn't matter what I say I'm always wrong. Alot of people making alot of assumptions about the goals based on nothing.

The dude said he wanted to make a helper function to return the last char of a c string. My last post meets all those requirements. Outside of that is just speculation.

[lastchance]

char* find_last(char s[], char c)
{
   if ( *s == '\0' ) return nullptr;
   char *last = find_last( s + 1, c );
   return last ? last : ( *s == c ? s : nullptr );
}

[dhayden]

I think the straightforward approach is best here:

char* find_last(char s[], char c)
{
    char *result = nullptr;
    for (; *s; ++s) {
        if (*s == c) result = s;
    }
    return result;
}

[dakotad8218]

Thanks TheToaster. To everyone else I appreciate the info and will definitely implement in future programs but the exercise was just written the way it was written. As a beginner whose only prior programming experience was with web languages like PHP, strings are inherently easier for me to use and I'd much rather do that than deal with character arrays, but this exercise specifically asked me to use char x[] instead of string x. I'mma go ahead and mark this one solved. Thanks!