Sign in to follow this  

How Can I Avoid Repetitive Programming

This topic is 4301 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

is there like an alternative for this?
cin >> yes;
if ( yes == "yes" ){
  for(int i=0; i<=1000; i++){
    many many commands, including changing variables.
  }
}
else{
  for(int i=1000; i>=0; i--){
    exactly the same commands as above, but only in descending order.
  }
}


i want to only write it once! and I am having syntax error on the following code.
for (multimap<string, int, case_insensitive_test>::const_reverse_iterator const_it = multimap_order.rbegin() const; const_it != multimap_order.rend() const; ++const_it ){

or

for (multimap<string, int, case_insensitive_test>::const_reverse_iterator const_it = multimap_order.rbegin(); const_it != multimap_order.rend(); ++const_it ){

both end up as errors.

Share this post


Link to post
Share on other sites
I guess you can either do something like this:


char cBuffer = 'y';

for(int iIndex = cBuffer == 'y' ? 0 : 1000 ; cBuffer == 'y' ? iIndex <= 1000 : iIndex >= 0 ; cBuffer == 'y' ? iIndex ++ : iIndex --)
{
//slow though...
}







or write a function that does your stuff, and call it in the two for-loops...

Share this post


Link to post
Share on other sites
In order to help us help you... 1) What are you attempting to do with the "for" statements? (i.e. what are you trying to accomplish) 2) could you post the errors you're getting at those lines?

Share this post


Link to post
Share on other sites
You can do:

for (int i = 0; i <= 1000; i++)
{
j = i;
if (yes != "yes")
j = 1000 - i;

// ... use j instead of i in your code inside the for loop ...
}



You do suffer the comparison and possible branch penalty inside your loop for doing it this way, but you do only have to write your code once.

Maybe it might be better to use a #define?

Share this post


Link to post
Share on other sites

Here is a quick cheesy way to do the first:
cin >> yes;
int direction, start, end;
if ( yes == "yes" ){
direction = 1;
start = 0;
end = 1000;
} else {
direction = -1;
start = 1000;
end = 0;
}

for (int i = start; i != end; i += direction) {
// code
}


You could manage it with just direction, but that would be annoying.

Second question. First, please use typedefs and keep your lines shorter! Long lines are very much a write-only-code style.

Rewritten:
Quote:
typedef multimap<string, int, case_insensitive_test> my_map;
typedef my_map::const_reverse_iterator my_it;
for (my_it const_it = multimap_order.rbegin() const;
const_it != multimap_order.rend() const;
++const_it ){


or
typedef multimap<string, int, case_insensitive_test> my_map;
typedef my_map::const_reverse_iterator my_it;
for ( my_it const_it = multimap_order.rbegin();
const_it != multimap_order.rend();
++const_it ){


What is the type of multimap_order?

Share this post


Link to post
Share on other sites
Quote:

i want to only write it once!


Isn't that the reason functions we're invented in the 1950s?

Seriously though, what in the world are you talking about. Use a function, an inline function, a functor, a macro ... whatever. That's what a programming langauge is for, organizing reused code into blocks.

Share this post


Link to post
Share on other sites

multimap<string, int, case_insensitive_test> multimap_order;


this is the type.


This is line 3638:

for (multimap<string, int, case_insensitive_test>::const_reverse_iterator const_it = multimap_order.rbegin(); const_it != multimap_order.rend(); ++const_it ){


[2], int, std::string (*)[2], int, std::string)':
shenuadmin.cpp:3638: error: no match for 'operator!=' in 'const_it != std::multimap<_Key, _Tp, _Compare, _Alloc>::rend() [with _Key = std::string, _Tp = int, _Compare = case_insensitive_test, _Alloc = std::allocator<std::pair<const std::string, int> >]()'


and my question #1.

cin >> yes;
if ( yes == "yes" ){
for(int i=0; i<=1000; i++){
many many commands, including changing variables.
}
}
else{
for(int i=1000; i>=0; i--){
exactly the same commands as above, but only in descending order.
}
}


I thought writing a sample simple example would simplify the answer, but I guess not.


if ( sort_type == "descending" ){
for (multimap<string, int, case_insensitive_test>::const_iterator const_it = multimap_order.begin(); const_it != multimap_order.end(); ++const_it ){
//BLA BLA BLA
}
}
else{
for (multimap<string, int, case_insensitive_test>::const_reverse_iterator const_it = multimap_order.rbegin(); const_it != multimap_order.rend(); ++const_it ){
/// BLA BLA BLA
}
}



and in the BLABLABLA area,
I would like to put this in

Menu_Cfg="system/users/orders/";
Menu_Cfg+=ToString((*const_it).second);
Menu_Cfg+=".cgi";
count_order_flag++;
cout << " <TR BORDERCOLORDARK=\"WHITE\" BORDERCOLORLIGHT=\"#C6C3C6\" BORDERCOLOR=\"WHITE\" onClick='changeRow_order(this); change_OptionIconsOrder(\"" << (*const_it).second << "\", \"" << Temp_Select_All_Order_Number_Holder << "\", \"" << Temp_Select_All_Order_Holder << "\", \"type1\");' onDblClick='edit_order(\"" << (*const_it).second << "\");' id='changeme_order";
cout << (*const_it).second;
cout << "' style='cursor:default' onselectstart=\"return false\">";
cout << " <TD WIDTH=1 title=\"Marked Deletion\" NOWRAP>";
cout << " <INPUT TYPE=checkbox NAME='order_multidelete" << (*const_it).second << "' VALUE=1>";
cout << " </TD>";

cout << " <TD>";
if ( ReadPosition("status", Menu_Cfg) != "" ){
cout << "<IMG name=orderStatusIcon src=\"system/image/admin/menu/" << ReadPosition("status", Menu_Cfg) << ".gif\" width=16 height=16 align=absmiddle border=0>";
}
else{
cout << "&nbsp;";
}
cout << " </TD>";

cout << " <TD>";
cout << " &nbsp;" << ReadPosition("order_number", Menu_Cfg);
cout << " </TD>";

cout << " <TD>";
cout << " &nbsp;" << ReadPosition("customer_FirstName", Menu_Cfg);
cout << " </TD>";

cout << " <TD>";
cout << " &nbsp;" << ReadPosition("customer_LastName", Menu_Cfg);
cout << " </TD>";

cout << " <TD ALIGN=center>";
cout << " &nbsp;" << ReadPosition("total_items", Menu_Cfg);
cout << " </TD>";

cout << " <TD>";
cout << " &nbsp;<script>document.write(ToDigits(\"" << ReadPosition("subtotal", Menu_Cfg) << "\"));</script>";
cout << " </TD>";

cout << " <TD ALIGN=left>";
cout << " &nbsp;" << ReadPosition("status", Menu_Cfg);
cout << " </TD>";

cout << " <TD>";
cout << " &nbsp;" << ReadPosition("date", Menu_Cfg);
cout << " &nbsp;" << ReadPosition("time", Menu_Cfg);
cout << " </TD>";

Share this post


Link to post
Share on other sites
you are right, i should have used functions.
and I was thinking about functions, but just didn't know how that would be applied.

I think the best approach would be....


function new_function(string Menu_Cfg){
//bbla blla bbal
}

...
...
if( type=="ascending" ){
for( iterator.begin; iter.end; iter++ ){
Menu_Cfg="system/something";
Menu_Cfg+=iter.first();
Menu_Cfg+=".cgi";

new_function(Menu_Cfg);
}
}
else{
for( iterator.rbegin; iter.rend; iter++ ){
Menu_Cfg="system/something";
Menu_Cfg+=iter.first();
Menu_Cfg+=".cgi";

new_function(Menu_Cfg);
}
}



I'm such an idiot!
I was doubting myself before I even let my brain to think.

Share this post


Link to post
Share on other sites
Best (most elegant and short to write, but not super efficient) solution I can think of for your original problem:



bool yes;
...
//somewhere decide if yes is 1 or 0
...
for(int i=(1000*(yes^1)); (i*((yes)?1:-1))<=(1000*(yes)); i+=((yes)?1:-1)){
cout << i << "\t";
}









edit: an issue with <= just a sec... ... FIXED with (i*((yes)?1:-1))

[Edited by - M2tM on March 2, 2006 8:20:32 PM]

Share this post


Link to post
Share on other sites
the original problem, in it's actual source code is something like this:

if ( sort_type == "descending" ){
for (multimap<string, int, case_insensitive_test>::const_iterator const_it = multimap_order.begin(); const_it != multimap_order.end(); ++const_it ){
//BLA BLA BLA
}
}
else{
for (multimap<string, int, case_insensitive_test>::const_reverse_iterator const_it = multimap_order.rbegin(); const_it != multimap_order.rend(); ++const_it ){
/// BLA BLA BLA
}
}





I tried to keep it simple, by making the it into an array, but in actuality it was an iterator. oops... sorry, i thought that would simplify matters, but I keep getting wrongs answers because I have asked the wrong question.

Sorry!!
but again, in order to solve the looping problem
I must first resolve the const_reverse_iterator problem :(

Share this post


Link to post
Share on other sites

This is line 3638:

for (multimap<string, int, case_insensitive_test>::const_reverse_iterator const_it = multimap_order.rbegin(); const_it != multimap_order.rend(); ++const_it ){



[2], int, std::string (*)[2], int, std::string)':
shenuadmin.cpp:3638: error: no match for 'operator!=' in 'const_it != std::multimap<_Key, _Tp, _Compare, _Alloc>::rend() [with _Key = std::string, _Tp = int, _Compare = case_insensitive_test, _Alloc = std::allocator<std::pair<const std::string, int> >]()'


multimap<string, int, case_insensitive_test> multimap_order;

and this is the type of maporder

Share this post


Link to post
Share on other sites
Quote:
Original post by M2tM
I had a bug and fixed it in my previous post, it's updated above.


would that method be better than passing to functions?
and i can not

i++

for I am using a map, and more specifically a const_reverse_iterator map that I can't even compile

Share this post


Link to post
Share on other sites
No, forget my example for what you're doing, I wrote that as something to replace these examples:


cin >> yes;
if ( yes == "yes" ){
for(int i=0; i<=1000; i++){
many many commands, including changing variables.
}
}
else{
for(int i=1000; i>=0; i--){
exactly the same commands as above, but only in descending order.
}
}





This one is the most efficient.

if ( yes == "yes" ){
direction = 1;
start = 0;
end = 1000;
} else {
direction = -1;
start = 1000;
end = 0;
}

for (int i = start; i != end; i += direction) {
// code
}





for (int i = 0; i <= 1000; i++)
{
j = i;
if (yes != "yes")
j = 1000 - i;

// ... use j instead of i in your code inside the for loop ...
}





Mine is closest to this one:

char cBuffer = 'y';

for(int iIndex=cBuffer=='y'?0:1000;cBuffer=='y'?iIndex<=1000:iIndex>=0;cBuffer== 'y'?iIndex++:iIndex--)
{
//slow though
}




and here is my example again:


bool yes;
... //somewhere set yes to equal 1 or 0.
... //1 will make it print 0 to 1000, 0 will print 1000 to 0
for(int i=(1000*(yes^1)); (i*((yes)?1:-1))<=(1000*(yes)); i+=((yes)?1:-1)){
cout << i << "\t";
}




All of these examples do the same iteration of either 1000 to 0 or 0 to 1000. Mine is the shortest to type and makes use of the binary nature of your yes/no variable. But it doesn't solve your real problem that was recently revealed. Sorry.

You have the right idea with your iterator reversing and putting it in a function.

Share this post


Link to post
Share on other sites
there is like no information on the internet about the const_reverse_iterators. well I've seen several examples, but I am using those examples and they don't seem to work.

M2tM

Thanks very much for you effort, looks like I can use those codes right away as soon as I solve my map iterator problem.

Share this post


Link to post
Share on other sites
Instead of just pulling the common code out of the loop, pull the loop out as well. Then pass the loop (in the form of a function pointer or function object) to the common code function as another argument.

Share this post


Link to post
Share on other sites
Tradone, I would suggest to do typedefs, like NotAYacc suggested. That will not only save you a lot of typing, but also a lot of typo opportunities. :) And it makes your code a bit more readable.

I just tried the following code in my compiler:


#include <map>

typedef std::multimap<std::string, int> mymap;
typedef mymap::const_reverse_iterator myiter;

int main(int argc, const char** argv)
{
mymap multimap_order;

for (myiter const_it = multimap_order.rbegin();
const_it != multimap_order.rend(); const_it++)
{
}

return 0;
}






It looks to me to be pretty much what you have, and it compiles just fine. (Well, it appears to be the same, except for the case-insensitive string comparison bit. I couldn't find that anywhere at the first glance, but then I am really not very well acquainted with the STL, so maybe I am missing something, but it shouldn't matter much, as with a typedef, if you decide to change the type of comparison, all you need to do is just change it in one place, and it will automagically work everywhere.) The fact that it looks the same and compiles fine means you probably have a typo in there somewhere (or the problem lies elsewhere). With typedefs, however, you would be safeguarded against those.

Share this post


Link to post
Share on other sites
Quote:
Original post by bballmitch
int a = strcmp(yes,"yes");


Actually, for std::strings, the == operator is overloaded and is what is supposed to be used for equality tests. Either that or the std::string.compare function should be used, unless sort_type in his actual code is a char*.

Vovan

Share this post


Link to post
Share on other sites
Quote:
Original post by M2tM
for(int i=(1000*(yes^1)); (i*((yes)?1:-1))<=(1000*(yes)); i+=((yes)?1:-1)){
cout << i << "\t";
}
Did the cat throw that up or something?[grin]

Share this post


Link to post
Share on other sites
Quote:
Original post by vovansim
Tradone, I would suggest to do typedefs, like NotAYacc suggested. That will not only save you a lot of typing, but also a lot of typo opportunities. :) And it makes your code a bit more readable.

I just tried the following code in my compiler:

*** Source Snippet Removed ***

It looks to me to be pretty much what you have, and it compiles just fine. (Well, it appears to be the same, except for the case-insensitive string comparison bit. I couldn't find that anywhere at the first glance, but then I am really not very well acquainted with the STL, so maybe I am missing something, but it shouldn't matter much, as with a typedef, if you decide to change the type of comparison, all you need to do is just change it in one place, and it will automagically work everywhere.) The fact that it looks the same and compiles fine means you probably have a typo in there somewhere (or the problem lies elsewhere). With typedefs, however, you would be safeguarded against those.



#include <map>

typedef std::multimap<std::string, int> mymap;
typedef mymap::const_reverse_iterator myiter;

int main(int argc, const char** argv)
{
mymap multimap_order;

for (myiter const_it = multimap_order.rbegin();
const_it != multimap_order.rend(); const_it++)
{
}

return 0;
}


The code that you have provided me gave me this error:
151# g++ -o map_test2 map_test2.cpp
map_test2.cpp: In function `int main(int, const char**)':
map_test2.cpp:11: error: no match for 'operator!=' in 'const_it != std::multimap<_Key, _Tp, _Compare, _Alloc>::rend() [with _Key = std::string, _Tp = int, _Compare = std::less<std::string>, _Alloc = std::allocator<std::pair<const std::string, int> >]()'
map_test2.cpp:16:2: warning: no newline at end of file
/usr/include/c++/3.4/bits/stl_pair.h: At global scope:
/usr/include/c++/3.4/bits/stl_pair.h: In instantiation of `std::pair<const std::string, int>':
/usr/include/c++/3.4/bits/stl_tree.h:135: instantiated from `std::_Rb_tree_node<std::pair<const std::string, int> >'
/usr/include/c++/3.4/bits/stl_tree.h:1064: instantiated from `void std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_erase(std::_Rb_tree_node<_Val>*) [with _Key = std::string, _Val = std::pair<const std::string, int>, _KeyOfValue = std::_Select1st<std::pair<const std::string, int> >, _Compare = std::less<std::string>, _Alloc = std::allocator<std::pair<const std::string, int> >]'
/usr/include/c++/3.4/bits/stl_tree.h:565: instantiated from `std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::~_Rb_tree() [with _Key = std::string, _Val = std::pair<const std::string, int>, _KeyOfValue = std::_Select1st<std::pair<const std::string, int> >, _Compare = std::less<std::string>, _Alloc = std::allocator<std::pair<const std::string, int> >]'
map_test2.cpp:8: instantiated from here
/usr/include/c++/3.4/bits/stl_pair.h:73: error: `std::pair<_T1, _T2>::first' has incomplete type
/usr/include/c++/3.4/bits/stringfwd.h:56: error: declaration of `const struct std::string'
151#
151# gcc -v
Using built-in specs.
Configured with: FreeBSD/i386 system compiler
Thread model: posix
gcc version 3.4.2 [FreeBSD] 20040728
151#


Share this post


Link to post
Share on other sites
so I add the string header file


#include <map>
#include <string>

typedef std::multimap<std::string, int> mymap;
typedef mymap::const_reverse_iterator myiter;

int main(int argc, const char** argv)
{
mymap multimap_order;

for (myiter const_it = multimap_order.rbegin();
const_it != multimap_order.rend(); const_it++)
{
}

return 0;
}


151# g++ -o map_test2 map_test2.cpp
map_test2.cpp: In function `int main(int, const char**)':
map_test2.cpp:12: error: no match for 'operator!=' in 'const_it != std::multimap<_Key, _Tp, _Compare, _Alloc>::rend() [with _Key = std::string, _Tp = int, _Compare = std::less<std::string>, _Alloc = std::allocator<std::pair<const std::string, int> >]()'

Same error.

Share this post


Link to post
Share on other sites
Quote:
Original post by Tradone
151# g++ -o map_test2 map_test2.cpp
map_test2.cpp: In function `int main(int, const char**)':
map_test2.cpp:12: error: no match for 'operator!=' in 'const_it != std::multimap<_Key, _Tp, _Compare, _Alloc>::rend() [with _Key = std::string, _Tp = int, _Compare = std::less<std::string>, _Alloc = std::allocator<std::pair<const std::string, int> >]()'


Dude.

Section 23.1.2 (3) of ISO/IEC 14882:1998(E). Read it and weep.

A map::iterator isn't going to support operator==() because it uses strict weak ordering. Try using operator<().

for (myiter const_it = multimap_order.rbegin();
const_it < multimap_order.rend();
++const_it)
{
// ...
}

Share this post


Link to post
Share on other sites

This topic is 4301 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this