I would really appreciate some good advice on how to refactor a small app:
It basically reads a CSV (energy/heat values?), computes some stuff, and then prepares some data which should later be sent to GNUplot.
For example, I'm unsure about this:
Is it good practice to send variables into a function as pointers and manipulate them within the function body? Being more familiar with OOP (specifically Ruby), I'm used to being very careful with such things and try to change variables only by sending them to some method and assigning the method's return value to the variable again, e.g. x = sum(x, y)
(or with !
methods).
I'm sure there is a lot more to optimize in the code. I would be thankful for some basic hints on how to optimize the code. It doesn't have to be perfect, but it's quite a mess at the moment, and before refactoring it into the "wrong" direction, it would be nice to get some feedback here.
/*
============================================================================
Name : Energie.c
Author : Benjamin Pfammatter
Version : 0.1
Copyright : Your copyright notice
Description : Energy in C, Ansi-style
============================================================================
*/
#include <stdio.h>
#include <stdlib.h>
#include "Datenstruktur.h"
//#include "logger.h"
int main(void) {
item *myList = NULL; //Erzeugen einen Pointer vom Type item
plot *myList2 = NULL; //Erzeugen einen Pointer vom Type plot
int bar = 0;
int counter = 0;
double E_max = 0;
double E_min = 0;
double range = 0;
double width = 0;
printf("########## Dynamische Liste ##########\n\n");
printf("-- erzeuge leere Liste\n");
myList = initList(myList); //initialisiere den neuen Pointer, indem er auf NULL gesetzt wird
//LOG_PRINT("-- initList(): myList");
printf("-- pruefe auf leere Liste\n");
printIsemtyList(myList);
//LOG_PRINT("-- printIsemptyList(): myList ist leer");
printf("-- Einlesen von Daten\n");
myList = einlesensortiert(myList);
printf("########## Liste vorwaerts ausgeben ##########\n");
printListForward(myList);
//LOG_PRINT("++ printListForward()");
printf("########## Anzahl Elemente: ##########\n");
counter = countItems(myList);
printf("++%d\n", counter);
printf("########## Groesster Wert der Liste ist: ##########\n");
E_max = getEnergyMax(myList);
printf("++%lf\n", E_max);
printf("########## Kleinster Wert der Liste ist: ##########\n");
E_min = getEnergyMin(myList);
printf("++%lf\n", E_min);
printf("########## Der Messbereich betraegt: ##########\n");
range = effective_range(E_max, E_min);
printf("++%lf\n", range);
/*Wenn R¸ckgabe Wert = 0 muss neue Abfrage gemacht werden!!!!!!!*/
printf("########## Anzahl Balken: ##########\n");
bar = number_of_bars(counter);
printf("++%d\n", bar);
printf("########## Abschnittswert: ##########\n");
width = section_value(range, bar);
printf("++%lf\n", width);
printf("########## Neue Dynamische Liste ##########\n\n");
printf("-- erzeuge leere Liste\n");
myList2 = initList2(myList2);
printf("-- pruefe auf leere Liste\n");
printisemptyList2(myList2);
printf("-- sortiere Liste\n");
myList2 = sortierenfuergnu(myList2, myList, width, E_max, E_min, counter,
bar);
printf("-- Liste ausgeben\n");
printListForward2(myList2);
//printf("++test for schleife\n");
//probeforsch( E_min, width, E_max);
//printf("%lf\n",i);
return EXIT_SUCCESS;
}
Other file:
/*
* Datenstruktur.c
*
* Created on: 30.12.2013
* Author: Captain Morgan
*/
#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#define MAX 20
/*--------------------------------------------------*/
struct daten {
char date[MAX];
char time[MAX];
double spannung;
double strom;
double wirkfaktor;
double energie;
struct daten *next;
};
/*--------------------------------------------------*/
typedef struct daten item;
/*--------------------------------------------------*/
item *initList(item *list) {
return (list = NULL);
}
/*--------------------------------------------------*/
int isemptyList(item *list) {
return (list == NULL);
}
/*--------------------------------------------------*/
void printIsemtyList(item *list) {
item *iter_List = list;
if (isemptyList(iter_List))
printf("++ Liste ist leer\n");
else
printf("++ Liste ist nicht leer\n");
}
/*--------------------------------------------------*/
//FILE dateiOpen(void) {
// FILE *datei;
//
// datei = fopen("Maschine_02.csv","r");
// if (NULL == datei) {
// printf("Konnte Datei nicht ˆffnen!\n");
// return (*datei);
// }
// return (*datei);
//}
/*--------------------------------------------------
Sortiert einlesen und Berechnen der Energie
Sortiert nach der Energie. Von klein nach gross.
--------------------------------------------------*/
item *einlesensortiert(item *list) {
item *tmp;
FILE *datei;
item *iter_List = list;
datei = fopen("Maschine_01.csv", "r");
if (NULL == datei) {
printf("Konnte Datei nicht ˆffnen!\n");
return (0);
}
while (!feof(datei)) {
tmp = (item *) malloc(sizeof(item));
fscanf(datei, "%s \t %s \t %lf \t %lf \t %lf\n", tmp->date, tmp->time,
&tmp->spannung, &tmp->strom, &tmp->wirkfaktor);
tmp->energie = ((sqrt(3)) * (tmp->spannung) * (tmp->strom) * cos(
(tmp->wirkfaktor))) * 1; //1 f¸r Sek.
// P(Leistung) = (Wurzel3) * Strom * Spannung / W (Energie) P * t (in sek)
tmp->next = NULL;
if (isemptyList(iter_List) || tmp->energie < iter_List->energie) {
tmp->next = iter_List;
list = tmp;
} else {
while (iter_List->next != NULL && tmp->energie
> iter_List->next->energie)
iter_List = iter_List->next;
tmp->next = iter_List->next;
iter_List->next = tmp;
}
iter_List = list;
}
return (list);
}
/*--------------------------------------------------*/
void printListForward(item *list) {
item *iter_List = list;
if (iter_List) {
printf("++ %s\t", iter_List->date);
printf("++ %s\t", iter_List->time);
printf("++ %lf\t", iter_List->spannung);
printf("++ %lf\t", iter_List->strom);
printf("++ %lf\t", iter_List->wirkfaktor);
printf("++ %lf\n", iter_List->energie);
printListForward(iter_List->next);
}
}
/*--------------------------------------------------*/
int countItems(item *list) {
item *iter_List = list;
int counter = 0;
while (iter_List) {
counter++;
iter_List = iter_List->next;
}
return (counter);
}
/*--------------------------------------------------
Achtung funktioniert nur mit positiven Werten!!!
--------------------------------------------------*/
double getEnergyMax(item *list) {
item *iter_List = list;
double tmp = 0;
while (iter_List != NULL) {
if (iter_List->energie > tmp) {
tmp = iter_List->energie;
}
iter_List = iter_List->next;
}
return (tmp);
}
/*--------------------------------------------------*/
double getEnergyMin(item *list) {
item *iter_List = list;
double tmp = iter_List->energie;
while (iter_List != NULL) {
if (iter_List->energie < tmp) {
tmp = iter_List->energie;
}
iter_List = iter_List->next;
}
return (tmp);
}
/*--------------------------------------------------*/
double effective_range(double E_max, double E_min) {
double range = 0;
range = E_max - E_min;
if (range == 0) //Wenn die Differenz gleich 0 ist, wird 0 zur¸ckgegeben. Mit Kevin anschauen welchen Wert er will!!
return 0;
else
return (range);
}
/*--------------------------------------------------*/
int number_of_bars(int counter) {
if (counter < 5)
return (0);
else if (counter <= 10)
return (2);
else if (counter <= 100)
return (5);
else
return (10);
}
/*--------------------------------------------------*/
double section_value(double range, int bar) {
double width = 0;
width = range / (bar);
return (width);
}
/*--------------------------------------------------*/
struct gnuplot {
int yAchse;
double xAchse;
double prozent;
struct gnuplot *next;
};
/*--------------------------------------------------*/
typedef struct gnuplot plot;
/*--------------------------------------------------*/
plot *initList2(plot *list2) {
return (list2 = NULL);
}
/*--------------------------------------------------*/
int isemptyList2(plot *list2) {
return (list2 == NULL);
}
/*--------------------------------------------------*/
void printisemptyList2(plot *list2) {
plot *iter_List2 = list2;
if (isemptyList2(iter_List2))
printf("++Liste ist leer\n");
else
printf("++Liste ist nicht leer\n");
}
/*--------------------------------------------------*/
plot *sortierenfuergnu(plot *list2, item *list, double width, double E_max,
double E_min, int counter, int bar) {
item *iter_List = list;
plot *iter_List2 = list2;
plot *tmp2;
double i;
double b = E_min + width;
int counter2;
int z = 0;
while (z < bar) {
for (i = b; i < E_max; i += width) {
z++;
tmp2 = (plot *) malloc(sizeof(plot));
tmp2->xAchse = i;
counter2 = 0;
while (i >= iter_List->energie) {
counter2++;
//printf("%lf\t",i);
//printf("%lf\n",iter_List->energie);
iter_List = iter_List->next;
//printf("\t\t%lf\n",iter_List->energie);
}
tmp2->yAchse = counter2;
tmp2->prozent = (100 / counter) * counter2;
tmp2->next = NULL;
if (isemptyList2(iter_List2))
list2 = tmp2;
else {
while (iter_List2->next != NULL)
iter_List2 = iter_List2->next;
iter_List2->next = tmp2;
}
iter_List2 = list2;
}
}
return (list2);
}
/*--------------------------------------------------*/
void printListForward2(plot *list2) {
plot*iter_List2 = list2;
if (iter_List2) {
printf("++ %d\t", iter_List2->yAchse);
printf("++ %lf\t", iter_List2->xAchse);
printf("++ %.2lf%%\n", iter_List2->prozent);
printListForward2(iter_List2->next);
}
}
/*--------------------------------------------------*/
double probeforsch(double E_min, double width, double E_max) {
double i;
double b = E_min + width;
for (i = b; i < E_max; i += width) {
printf("%lf\n", i);
}
return (i);
}
Code can also be downloaded from here.